On 11/9/21 8:24 PM, Numan Siddique wrote: > On Fri, Nov 5, 2021 at 5:21 AM Dumitru Ceara <dce...@redhat.com> wrote: >> >> There are various costs (e.g., not being able to perform hardware >> offload in some cases) when using check_pkt_larger() so the CMS >> can now limit the impact by bypassing the packet length checks for >> specific types of traffic (e.g., TCP). >> >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2011779 >> Signed-off-by: Dumitru Ceara <dce...@redhat.com> > > > Hi Dumitru, >
Hi Numan, Thanks for reviewing this! > Thanks for the patch. Instead of adding a bypass option, how about adding > the option to check for the extra matches ? We could do that too, even if it's just for making it more natural to configure. > > i.e new option can be - gateway_mtu_match. And ovn-northd would append > this match to the existing logical flows with the action - check_pkt_larger ? That wouldn't work directly. That's because we already append check_pkt_larger() to flows that perform other actions. Let's assume we start with gateway_mtu not configured, then we'd have (at least) the following flows: - table=rtr_in_admission,prio=50, eth.mcast && inport == <rtr-port>, actions='REG_INPORT_ETH_ADDR = <rtr-port>.eth, next;' - table=rtr_in_admission,prio=50, eth.dst == <rtr-port> && inport == <rtr-port> && is_chassis_resident(<rtr-port>), actions='REG_INPORT_ETH_ADDR = <rtr-port>.eth, next;' If we now configure gateway_mtu=1400 these flows get changed to: - table=rtr_in_admission,prio=50, eth.mcast && inport == <rtr-port>, actions='REG_INPORT_ETH_ADDR = <rtr-port>.eth; reg9[1]=check_pkt_larger(1428); next;' - table=rtr_in_admission,prio=50, eth.dst == <rtr-port> && inport == <rtr-port> && is_chassis_resident(<rtr-port>), actions='REG_INPORT_ETH_ADDR = <rtr-port>.eth; reg9[1]=check_pkt_larger(1428); next;' Just adding an extra gateway_mtu_match to the matches of the two flows above would be wrong as we would not be performing the other actions ("REG_INPORT_ETH_ADDR = <rtr-port>; next;") for all traffic that doesn't need check_pkt_larger. > > This would not increase the number of lflows ? It's possible that I may have > missed a few details/scenarios. What do you think? Due to the reasons above, this other approach you're suggesting would unfortunately also have to increase the number of flows. However, we're not talking about a huge increase because we currently have at most 3 flows per logical router port that has gateway_mtu enabled. I don't expect the number of such ports to be huge. > > Otherwise the patch LGTM. > If you think it makes more sense to configure "gateway_mtu_match" instead of "gateway_mtu_bypass", I'm ok with that and I can send a v3 but the implementation would be very similar to the one in v2. Thanks, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev