Skip to content

Conversation

Cristib05
Copy link
Contributor

There are some cases when OpenThread opens a sockets and doesn't choose as default it's internal interface, this leading to usage of platform UDP module which will then send back the packet to the OpenThread interface. In this case, the packet should not be treated as originated from backbone interface.

Backbone router multicast listener callback functionality is improved. A route with a prefix length of 128 is set
and a multicast address is added for each listener registration. OpenThread interface joins that multicast address group.

Enabled forwarding capabilities for Backbone interface. A border router should be able to perform default packet forwarding for destination addresses with a multicast scope greater than admin-local. In order to achieve this, multicast routes have been added to those addreses. [https://datatracker.ietf.org/doc/rfc7346/]

For Border Router application, ip6_addr_cb is not installed. otIp6SubscribeMulticastAddress call would re-register an IPV6 multicast address which might have been registered by an OpenThread node using ipmadd add command and even if that node performed ipmaddr del, the address was still present in multicast listener table. This also led to a missing MLDv2 message with that specific multicast IPV6 address.

Comment on lines 586 to 592
"ff04::1",
/** Site-Local scope multicast address */
"ff05::1",
/** Organization-Local scope multicast address */
"ff08::1",
/** Global scope multicast address */
"ff0e::1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like overkill as you could create IPv6 address dynamically.
So something like

int mcast_groups[] = {
	/** Admin-Local scope multicast address */
	4,
	/** Site-Local scope multicast address */
	5,
	/** Organization-Local scope multicast address */
	8,
	/** Global scope multicast address */
	e,
};

ARRAY_FOR_EACH(mcast_groups, i) {
	struct in6_addr addr;

	net_ipv6_addr_create(&addr, (0xff << 8) | i, 0, 0, 0, 0, 0, 0, 1);

...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thanks!

@Cristib05 Cristib05 force-pushed the enhance_border_router_packet_forwarding branch from 3d4abde to 08b9ae7 Compare October 14, 2025 13:25
@Cristib05 Cristib05 force-pushed the enhance_border_router_packet_forwarding branch from 08b9ae7 to 141cd57 Compare October 15, 2025 05:14
Comment on lines 585 to 593
static int mcast_group_idx[] = {/** Admin-Local scope multicast address */
0x04,
/** Site-Local scope multicast address */
0x05,
/** Organization-Local scope multicast address */
0x08,
/** Global scope multicast address */
0x0e,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering the layout of this and why indent like that. Are you using clang-format for this? Asking because it looks weird to add comment right after starting block {, also makes line quite long.

Copy link
Contributor Author

@Cristib05 Cristib05 Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have clang-format 20.1.5 and I use scripts/ci/check_compliance.py. I created a new commit where I have moved the first comment to the next line. This was the output:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is very weird indeed, try putting the comment on the same line?

	static int mcast_group_idx[] = {
		0x04, /** Admin-Local scope multicast address */
		0x05, /** Site-Local scope multicast address */
		0x08, /** Organization-Local scope multicast address */
		0x0e, /** Global scope multicast address */
	};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified as per last example, no clang-format issues on my side, hopefully we have a winner :)

@Cristib05 Cristib05 force-pushed the enhance_border_router_packet_forwarding branch from 141cd57 to 709f956 Compare October 15, 2025 07:18
There are some cases when OpenThread opens a sockets and doesn't choose
as default it's internal interface, this leading to usage of
platform UDP module which will then send back the packet to the
OpenThread interface. In this case, the packet should not be treated as
originated from backbone interface.

Backbone router multicast listener callback functionality is improved.
A route with a prefix length of 128 is set
and a multicast address is added for each listener registration.
OpenThread interface joins that multicast address group.

Enabled forwarding capabilities for Backbone interface.
A border router should be able to perform default packet forwarding for
destination addresses with a multicast scope greater than admin-local.
In order to achieve this, multicast routes have been added to those
addreses. [https://datatracker.ietf.org/doc/rfc7346/]

For Border Router application, `ip6_addr_cb` is not installed.
otIp6SubscribeMulticastAddress call would re-register an IPV6 multicast
address which might have been registered by an OpenThread node using
`ipmadd add` command and even if that node performed `ipmaddr del`,
the address was still present in multicast listener table. This also
led to a missing MLDv2 message with that specific multicast IPV6
address.

Signed-off-by: Cristian Bulacu <[email protected]>
@Cristib05 Cristib05 force-pushed the enhance_border_router_packet_forwarding branch from 709f956 to 30a61b4 Compare October 15, 2025 07:40
Comment on lines +586 to +589
0x04, /** Admin-Local scope multicast address */
0x05, /** Site-Local scope multicast address */
0x08, /** Organization-Local scope multicast address */
0x0e, /** Global scope multicast address */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we need to have one more round. If you use this style, then the comment needs to be written with < so like this

		0x04, /**< Admin-Local scope multicast address */
		0x05, /**< Site-Local scope multicast address */
		0x08, /**< Organization-Local scope multicast address */
		0x0e, /**< Global scope multicast address */

otherwise the comment in documentation refers to the next line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this even parsed for documentation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, very good point. In that case we could just say /* xxxx */ without doxygen comment. If we keep doxygen comment, and want to document this, then we should do /**< xxxx */.
Anyway, as this is in .c file, it does not matter much, I was a bit too pedantic here.

Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants