[ovs-dev] [PATCH ovn v2] nb: Add support for gateway_mtu_bypass.

Numan Siddique numans at ovn.org
Wed Nov 10 18:22:51 UTC 2021


On Wed, Nov 10, 2021 at 12:24 PM Numan Siddique <numans at ovn.org> wrote:
>
> On Wed, Nov 10, 2021 at 5:19 AM Dumitru Ceara <dceara at redhat.com> wrote:
> >
> > On 11/9/21 8:24 PM, Numan Siddique wrote:
> > > On Fri, Nov 5, 2021 at 5:21 AM Dumitru Ceara <dceara at 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 <dceara at 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.
>
> Ah I see.  I missed this.  Thanks.
> >
> > 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.
>
> Agree.
>
> >
> > >
> > > 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.
>
> Having a whitelist match seems more natural to me.  So my suggestion
> would be for "gateway_mtu_match".

On second thought,  I think "gateway_mtu_bypass" makes more sense.
If a user wants to bypass "tcp" protocol,  then with gateway_mtu_match user has
to add a match like - "(udp || icmp)" which would result in more openflows.

I'll take a closer look into v2 tomorrow.

Thanks
Numan



>
> Thanks
> Numan
>
> >
> > Thanks,
> > Dumitru
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >


More information about the dev mailing list