On Fri, Aug 6, 2021 at 12:00 PM Numan Siddique <[email protected]> wrote:
>
> On Thu, Aug 5, 2021 at 1:10 PM Lorenzo Bianconi
> <[email protected]> wrote:
> >
> > > The commit 1c9e46ab5 removed the outport match from the lflows, which
> > > leads to a problem for gateway routers that have multiple ports
> > > configured with different MTUs. For example, R0 has port P1, P2 and
P3.
> > > P2 and P3 both have gateway_mtu configured: P2 mtu = 1400, P3 mtu =
1500.
> > > Below lflows are generated:
> > > table=16(lr_in_larger_pkts ), priority=150 , match=(inport ==
"P1" && ip4 && reg9[[1]] && reg9[[0]] == 0), action=(icmp4_error {...
icmp4.frag_mtu = 1400; next(pipeline=ingress, table=0); };)
> > > table=16(lr_in_larger_pkts ), priority=150 , match=(inport ==
"P1" && ip4 && reg9[[1]] && reg9[[0]] == 0), action=(icmp4_error {...
icmp4.frag_mtu = 1500; next(pipeline=ingress, table=0); };)
> > >
> > > These two lflows have exact same match, but different actions (with
> > > different MTUs). This will result in a random one gets installed by
> > > ovn-controller and the icmp4_error message may contain incorrect mtu.
> > > This patch fixes it by adding the outport back for these flows, so
that
> > > mtu that matches the outport setting is used in the generated icmp
error
> > > messages.
> > >
> > > Another problem of the commit is that the ddlog part used Flow instead
> > > of MeterFlow for the gateway router flows that generates icmp errors,
> > > while the flows for Distributed Gateway Ports use MeterFlow. This
patch
> > > also fixes that by combining the DGP and gateway router code using
> > > MeterFlow. The check for DGP and Gateway Router is removed to simplify
> > > the code, because checking the gateway_mtu config should be
sufficient,
> > > which also makes it consistent with the flows in the ADMISSION and
> > > IP_INPUT stages where we didn't check DGP and gateway router but only
> > > the gateway_mtu settings.
> > >
> > > Fixes: 1c9e46ab5 ("northd: add check_pkt_larger lflows for ingress
traffic")
> > > Signed-off-by: Han Zhou <[email protected]>
> >
> > Hi Han,
> >
> > I have not run any test yet, but the code seems fine to me.
> >
> > Regards,
> > Lorenzo
>
> Thanks for fixing this.
>
> Please see below one small nit comment.
>
> With that addressed:
>
> Acked-by: Numan Siddique <[email protected]>
>
> Numan
>
Thanks Numan. I applied the patch with the nit addressed.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
