[ip6] deliver multicast to host if not from there #9817
Conversation
bukepo
force-pushed
the
forward-to-host
branch
from
fea5444
to
70b4c85
Compare
Size Report of OpenThread
|
| @@ -1121,7 +1121,8 @@ Error Ip6::HandleDatagram(OwnedPtr<Message> aMessagePtr, const void *aLinkMessag | |||
| } | |||
| #endif | |||
|
|
|||
| forwardHost = header.GetDestination().IsMulticastLargerThanRealmLocal(); | |||
| // Forward multicast packets to host network stack unless they came from there. | |||
| forwardHost = !aMessagePtr->IsOriginHostUntrusted(); | |||
There was a problem hiding this comment.
The existing code would not apply any check on origin. So we would have forwarded a message originating from host back to host when GetDestination().IsMulticastLargerThanRealmLocal() and now we wont? Should we keep this behavior unchanged?
I think the main change we want is to allow all multicast traffic to be passed to host (not just IsMulticastLargerThanRealmLocal()), so how about this?
| forwardHost = !aMessagePtr->IsOriginHostUntrusted(); | |
| forwardHost = true; |
There was a problem hiding this comment.
@librasungirl I'm afraid passing a packet originated from host back to host would cause infinite loop or duplicate packets being forwarded. Do we have a known use case for delivering packets back to the host?
There was a problem hiding this comment.
We have APIs that allow caller to specify the "multicast loop" desired behavior (per messages) -> otMessageSetLoopbackToHostAllowed().
Do we have a known use case for delivering packets back to the host?
I don't know about exact use-cases but I expect there may be some.
- There were some discussions on this specific behavior here -> Potential incorrect handling of outgoing multicast packets #9489
- So I suggest not changing existing behavior, in particular regarding how the "larger than realm local" multicast msg from host are handled. Today, we allow them to be passed back to host (assuming
otMessageSetLoopbackToHostAllowedis set).
There was a problem hiding this comment.
For multicast loop, I think the host stack should handle it by enabling the IPV6_MULTICAST_LOOP on the socket that desires the behavior. We don't need to pass back a packet to the host again, as it may conflict with the host stack configuration. Thoughts?
There was a problem hiding this comment.
I checked otMessageSetLoopbackToHostAllowed() is currently used in src/posix/platform/netif.cpp. @superwhd @librasungirl @wgtdkp is this necessary? Actually I suspect this would result in duplicate messages (scope larger than realm local) being processed by sockets in the host.
There was a problem hiding this comment.
For multicast loop, I think the host stack should handle it by enabling the IPV6_MULTICAST_LOOP on the socket that desires the behavior.
I guess this can work for posix, but OT may be used with other OS/stacks. The #9489 talks about Zephyr and multicast use. I suggested a similar suggestion to what you have in mind here in this comment #9489 (comment) but the API is added now to support this.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9817 +/- ##
===========================================
+ Coverage 67.32% 81.97% +14.64%
===========================================
Files 485 524 +39
Lines 58556 66160 +7604
===========================================
+ Hits 39423 54232 +14809
+ Misses 19133 11928 -7205
|
|
@bukepo an idea/suggestion on this: I think this PR is doing two conceptual changes at the same time (they are actually part of the same line of code 😃):
How about breaking this into two separate PRs? I feel first one change is perfectly fine (it is added/new function/behavior ) but the second one may require more consideration (may conflict with |
Sounds good! Created #9824 as a temporary fix to the original issue in connectedhomeip. We can continue the discussion here. |
This commit filter out multicast traffic to host if they came from there, so that a multicast packet will not be processed twice on host.
This commit filter out multicast traffic to host if they came from there, so
that a multicast packet will not be processed twice on host.