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 > > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
