HI Ales,

Thanks for finding the issue, just one comment for you and Erlon.

On 1/26/26 3:24 PM, Erlon Rodrigues Cruz via dev wrote:
Hey Ales,

<snip>

I was able to figure out why is it happening, dunno if we should keep the
test around
but it can be reproduced even with multicast UDP traffic so it might be
enough to adjust
the UDP test to also send multicast.  The following two logical flows are
to "blame":


Well, I think the broader the coverage, the better. The advantage of having
a DHCP client/server is that the traffic will contain all the nuances of
real traffic, and we don't have to encode all protocol data. For example,
the return traffic should be difficult to generate since it will not be
multicast. I tried to reuse NETNS_START_DHCPD, but I encountered various
permission errors and couldn't get it to work. Dnsmasq seems easier to use
and doesn't rely on an external config file.


         /* Do not send multicast packets to conntrack. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "eth.mcast",
                       "next;", lflow_ref);
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, "eth.mcast",
                       "next;", lflow_ref);

We basically skip CT in case of multicast, and because the translation
adds new
prereq of +ct.trk we will never match that flow. I'm afraid that there
isn't any
easy/good solution. If we disable those flows we are imposing performance
penalty on multicast traffic. If we say that's acceptable we need to
clearly
state that too in the documentation it's in the same ballpark with "broken"
HW offload.


What if we discard it only for DHCP traffic? I got it passing with
something like this:

         if (acl_udp_ct_translation) {
             ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 120,
                           "eth.mcast && udp && "
                           "(udp.src == 67 || udp.src == 68 || "
                           "udp.dst == 67 || udp.dst == 68)",
                           REGBIT_CONNTRACK_DEFRAG" = 1; next;",
                           lflow_ref);
         }

This takes higher priority than the more general multicast rule, so
performance should not be a problem if we apply it for DHCP.

The other thing I noticed when looking at the code you highlighted was this section right above it:

         /* Ingress and Egress Pre-ACL Table (Priority 110).
          *
          * Not to do conntrack on ND and ICMP destination
          * unreachable packets. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
                       "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
"(udp && udp.src == 546 && udp.dst == 547)", "next;",
                       lflow_ref);
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
                       "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
"(udp && udp.src == 546 && udp.dst == 547)", "next;",
                       lflow_ref);

That seems to always allow DHCPv6 traffic with ports 546/547. I realize Erlon's proposal would only apply if the new flag was set, but is there a reason we shouldn't just bypass conntrack for IPv4 with ports 67/68 similarly?

Thanks,

-Brian
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to