On Fri, Sep 20, 2024 at 5:08 PM Numan Siddique <[email protected]> wrote:

> On Mon, Sep 9, 2024 at 5:07 PM Mark Michelson <[email protected]> wrote:
> >
> > Looks good to me, Ales.
> >
> > Acked-by: Mark Michelson <[email protected]>
>
> Thanks.  I applied both the patches in this series to main.
>
> Numan
>

Thank you Numan, could you also backport the first patch down to 23.09?

Regards,
Ales


> >
> > On 8/20/24 05:01, Ales Musil wrote:
> > > If multichassis VIF is set with "unknown" address it might store
> > > wrong MAC address into the FDB table. The ICMP Need frag is generated
> > > by swapping the MAC src and dst of the original packet, however the
> > > inport remains the same. As a consequence the match on the inport to
> > > learn FDB will store source MAC address which is the original
> > > destination address, that leads to redirection of traffic the VIF
> > > which is wrong.
> > >
> > > Swap the inport and outport for the ICMP error to prevent this issue
> > > it also makes more sense as the ICMP is supposed to be generated by
> > > entity along the way and not by the original VIF.
> > >
> > > Note that those flows is still needed as userspace datapath is not
> > > capable of PMTUD yet.
> > >
> > > Reported-at: https://issues.redhat.com/browse/FDP-620
> > > Signed-off-by: Ales Musil <[email protected]>
> > > ---
> > > v2: Rebase on top of latest main.
> > >      Fix the typo in the test and add note into the commit message.
> > > ---
> > >   controller/physical.c |  6 ++++++
> > >   tests/ovn.at          | 24 +++++++++++++++++++++---
> > >   2 files changed, 27 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/controller/physical.c b/controller/physical.c
> > > index 9e04ad5f2..55edbd54e 100644
> > > --- a/controller/physical.c
> > > +++ b/controller/physical.c
> > > @@ -1258,6 +1258,12 @@ reply_imcp_error_if_pkt_too_big(struct
> ovn_desired_flow_table *flow_table,
> > >       ofpact_put_set_field(
> > >           &inner_ofpacts, mf_from_id(MFF_LOG_FLAGS), &value, &mask);
> > >
> > > +    /* inport <-> outport */
> > > +    put_stack(MFF_LOG_INPORT, ofpact_put_STACK_PUSH(&inner_ofpacts));
> > > +    put_stack(MFF_LOG_OUTPORT, ofpact_put_STACK_PUSH(&inner_ofpacts));
> > > +    put_stack(MFF_LOG_INPORT, ofpact_put_STACK_POP(&inner_ofpacts));
> > > +    put_stack(MFF_LOG_OUTPORT, ofpact_put_STACK_POP(&inner_ofpacts));
> > > +
> > >       /* eth.src <-> eth.dst */
> > >       put_stack(MFF_ETH_DST, ofpact_put_STACK_PUSH(&inner_ofpacts));
> > >       put_stack(MFF_ETH_SRC, ofpact_put_STACK_PUSH(&inner_ofpacts));
> > > diff --git a/tests/ovn.at b/tests/ovn.at
> > > index 50c9f04da..bbb1bee35 100644
> > > --- a/tests/ovn.at
> > > +++ b/tests/ovn.at
> > > @@ -15740,14 +15740,17 @@
> m4_define([MULTICHASSIS_PATH_MTU_DISCOVERY_TEST],
> > >      second_mac=00:00:00:00:00:02
> > >      multi1_mac=00:00:00:00:00:f0
> > >      multi2_mac=00:00:00:00:00:f1
> > > +   external_mac=00:00:00:00:ee:ff
> > >      first_ip=10.0.0.1
> > >      second_ip=10.0.0.2
> > >      multi1_ip=10.0.0.10
> > >      multi2_ip=10.0.0.20
> > > +   external_ip=10.0.0.30
> > >      first_ip6=abcd::1
> > >      second_ip6=abcd::2
> > >      multi1_ip6=abcd::f0
> > >      multi2_ip6=abcd::f1
> > > +   external_ip6=abcd::eeff
> > >
> > >      check ovn-nbctl ls-add ls0
> > >      check ovn-nbctl lsp-add ls0 first
> > > @@ -15866,6 +15869,24 @@
> m4_define([MULTICHASSIS_PATH_MTU_DISCOVERY_TEST],
> > >
> > >      reset_env
> > >
> > > +   AS_BOX([Multi with "unknown" to external doesn't produce wrong
> FDB])
> > > +   len=3000
> > > +   check ovn-nbctl --wait=hv lsp-set-addresses multi1 unknown
> > > +
> > > +   packet=$(send_ip_packet multi1 1 $multi1_mac $external_mac
> $multi1_ip $external_ip $(payload $len) 1 ${expected_ip_mtu})
> > > +   echo $packet >> hv1/multi1.expected
> > > +
> > > +   packet=$(send_ip6_packet multi1 1 $multi1_mac $external_mac
> $multi1_ip6 $external_ip6 $(payload $len) 1 ${expected_ip_mtu})
> > > +   echo $packet >> hv1/multi1.expected
> > > +
> > > +   check_pkts
> > > +   reset_env
> > > +
> > > +   check_row_count fdb 0 mac="$external_mac"
> > > +   ovn-sbctl --all destroy fdb
> > > +
> > > +   check ovn-nbctl --wait=hv lsp-set-addresses multi1 "${multi1_mac}
> ${multi1_ip} ${multi1_ip6}"
> > > +
> > >      AS_BOX([Packets of proper size are delivered from multichassis to
> regular ports])
> > >
> > >      len=1000
> > > @@ -15965,9 +15986,6 @@
> m4_define([MULTICHASSIS_PATH_MTU_DISCOVERY_TEST],
> > >      packet=$(send_ip6_packet multi1 1 $multi1_mac $multi2_mac
> $multi1_ip6 $multi2_ip6 $(payload $len) 1)
> > >      echo $packet >> hv1/multi1.expected
> > >
> > > -   check_pkts
> > > -   reset_env
> > > -
> > >      AS_BOX([MTU updates are honored in ICMP Path MTU calculation])
> > >
> > >      set_mtu() {
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to