On Thu, Jan 29, 2026 at 2:59 PM Lorenzo Bianconi via dev < [email protected]> wrote:
> According RFC1812 Section 4.3.2.7 "An ICMP error message MUST NOT > be sent as the result of receiving a L2 broadcast packet". Drop related > flows. > > Reported-at: https://issues.redhat.com/browse/FDP-2652 > Fixes: 1c9e46ab5c05 ("northd: add check_pkt_larger lflows for ingress > traffic") > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > northd/northd.c | 10 +++++----- > northd/ovn-northd.8.xml | 12 ++++++------ > tests/ovn-northd.at | 13 ------------- > tests/ovn.at | 10 +++++----- > 4 files changed, 16 insertions(+), 29 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index e998c8081..086ecf183 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -14091,11 +14091,11 @@ build_adm_ctrl_flows_for_lrouter_port( > */ > ds_clear(match); > ds_put_format(match, "eth.mcast && inport == %s", op->json_key); > - build_gateway_mtu_flow(lflows, op, S_ROUTER_IN_ADMISSION, 50, 55, > - match, actions, &op->nbrp->header_, > - lflow_ref, > - REG_INPORT_ETH_ADDR " = %s; next;", > - op->lrp_networks.ea_s); > + ds_clear(actions); > + ds_put_format(actions, REG_INPORT_ETH_ADDR " = %s; next;", > + op->lrp_networks.ea_s); > + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ADMISSION, 50, > ds_cstr(match), > + ds_cstr(actions), lflow_ref, > WITH_HINT(&op->nbrp->header_)); > > ds_clear(match); > ds_put_cstr(match, "eth.dst == "); > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 0f6693b2f..279d81f35 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -2948,12 +2948,12 @@ output; > </p> > > <p> > - For a distributed logical router or for gateway router where > - the port is configured with <code>options:gateway_mtu</code> > - the action of the above flow is modified adding > - <code>check_pkt_larger</code> in order to mark the packet > - setting <code>REGBIT_PKT_LARGER</code> if the size is greater > - than the MTU. > + For unicast L2 traffic on a distributed logical router or for > + gateway router where the port is configured with > + <code>options:gateway_mtu</code> the action of the above flow > + is modified adding <code>check_pkt_larger</code> in order to > mark > + the packet setting <code>REGBIT_PKT_LARGER</code> if the size is > + greater than the MTU. > > If the port is also configured with > <code>options:gateway_mtu_bypass</code> then another flow is > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 3fa9d4b9e..512e42036 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -6866,7 +6866,6 @@ AT_CHECK_UNQUOTED([grep -e "chk_pkt_len" -e > "lr_in_larger_pkts" lr0flows | ovn_s > > AT_CHECK([grep -E "lr_in_admission.*check_pkt_larger" lr0flows | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public" && > is_chassis_resident("cr-lr0-public")), action=(reg9[[1]] = > check_pkt_larger(1514); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1514); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > ]) > > AT_CHECK([grep -E "lr_in_ip_input.*icmp4_error" lr0flows | > ovn_strip_lflows], [0], [dnl > @@ -6906,7 +6905,6 @@ AT_CHECK_UNQUOTED([grep -e "chk_pkt_len" -e > "lr_in_larger_pkts" lr0flows | ovn_s > > AT_CHECK([grep -E "lr_in_admission.*check_pkt_larger" lr0flows | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public"), action=(reg9[[1]] = > check_pkt_larger(1514); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1514); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > ]) > > AT_CHECK([grep -E "lr_in_ip_input.*icmp4_error" lr0flows | > ovn_strip_lflows], [0], [dnl > @@ -6943,9 +6941,7 @@ AT_CHECK_UNQUOTED([grep -e "chk_pkt_len" -e > "lr_in_larger_pkts" lr0flows | ovn_s > > AT_CHECK([grep "lr_in_admission" lr0flows | grep -e "check_pkt_larger" -e > "tcp" | ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public"), action=(reg9[[1]] = > check_pkt_larger(1514); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1514); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > table=??(lr_in_admission ), priority=55 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public" && (arp || tcp)), > action=(xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=55 , match=(eth.mcast && > inport == "lr0-public" && (arp || tcp)), action=(xreg0[[0..47]] = > 00:00:20:20:12:13; next;) > ]) > > # Set gateway_mtu option on lr0-sw0 > @@ -6982,8 +6978,6 @@ AT_CHECK_UNQUOTED([grep -e "chk_pkt_len" -e > "lr_in_larger_pkts" lr0flows | ovn_s > AT_CHECK([grep "lr_in_admission.*check_pkt_larger" lr0flows | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:00:00:ff:01 && inport == "lr0-sw0"), action=(reg9[[1]] = > check_pkt_larger(1414); xreg0[[0..47]] = 00:00:00:00:ff:01; next;) > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public"), action=(reg9[[1]] = > check_pkt_larger(1514); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1514); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-sw0"), action=(reg9[[1]] = check_pkt_larger(1414); > xreg0[[0..47]] = 00:00:00:00:ff:01; next;) > ]) > > AT_CHECK([grep -E "lr_in_ip_input.*icmp4_error" lr0flows | > ovn_strip_lflows], [0], [dnl > @@ -7033,12 +7027,8 @@ AT_CHECK_UNQUOTED([grep -e "chk_pkt_len" -e > "lr_in_larger_pkts" lr0flows | ovn_s > AT_CHECK([grep "lr_in_admission" lr0flows | grep -e "check_pkt_larger" -e > "tcp" | ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:00:00:ff:01 && inport == "lr0-sw0"), action=(reg9[[1]] = > check_pkt_larger(1414); xreg0[[0..47]] = 00:00:00:00:ff:01; next;) > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public"), action=(reg9[[1]] = > check_pkt_larger(1514); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1514); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-sw0"), action=(reg9[[1]] = check_pkt_larger(1414); > xreg0[[0..47]] = 00:00:00:00:ff:01; next;) > table=??(lr_in_admission ), priority=55 , match=(eth.dst == > 00:00:00:00:ff:01 && inport == "lr0-sw0" && (arp || tcp)), > action=(xreg0[[0..47]] = 00:00:00:00:ff:01; next;) > table=??(lr_in_admission ), priority=55 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public" && (arp || tcp)), > action=(xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=55 , match=(eth.mcast && > inport == "lr0-public" && (arp || tcp)), action=(xreg0[[0..47]] = > 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=55 , match=(eth.mcast && > inport == "lr0-sw0" && (arp || tcp)), action=(xreg0[[0..47]] = > 00:00:00:00:ff:01; next;) > ]) > > # Clear gateway_mtu option on lr0-public > @@ -7071,7 +7061,6 @@ ovn-sbctl dump-flows lr0 > lr0flows > > AT_CHECK([grep "lr_in_admission" lr0flows | grep -e "check_pkt_larger" | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public"), action=(reg9[[1]] = > check_pkt_larger(1518); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1518); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > ]) > > # tag 0 requires a parent port > @@ -7080,7 +7069,6 @@ check ovn-nbctl --wait=sb set Logical_Switch_Port > ext-port tag_request=0 > ovn-sbctl dump-flows lr0 > lr0flows > AT_CHECK([grep "lr_in_admission" lr0flows | grep -e "check_pkt_larger" | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public"), action=(reg9[[1]] = > check_pkt_larger(1514); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1514); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > ]) > > check ovn-nbctl --wait=sb set Logical_Switch_Port ext-port > parent_name=ext-parent-port > @@ -7088,7 +7076,6 @@ check ovn-nbctl --wait=sb set Logical_Switch_Port > ext-port parent_name=ext-paren > ovn-sbctl dump-flows lr0 > lr0flows > AT_CHECK([grep "lr_in_admission" lr0flows | grep -e "check_pkt_larger" | > ovn_strip_lflows], [0], [dnl > table=??(lr_in_admission ), priority=50 , match=(eth.dst == > 00:00:20:20:12:13 && inport == "lr0-public"), action=(reg9[[1]] = > check_pkt_larger(1518); xreg0[[0..47]] = 00:00:20:20:12:13; next;) > - table=??(lr_in_admission ), priority=50 , match=(eth.mcast && > inport == "lr0-public"), action=(reg9[[1]] = check_pkt_larger(1518); > xreg0[[0..47]] = 00:00:20:20:12:13; next;) > ]) > > check ovn-sbctl set chassis ch1 other_config:ct-commit-nat-v2=false > diff --git a/tests/ovn.at b/tests/ovn.at > index 4d15d4a62..4c1abb045 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -5872,7 +5872,7 @@ done > # ipv4 packet should be dropped for lp13 with mac f0000000113 > sip=192.168.0.13 > tip=192.168.0.23 > -test_ip 13 f0:00:00:00:01:13 f0:00:00:00:00:23 $sip $tip > +test_ip 13 f0:00:00:00:01:12 f0:00:00:00:00:23 $sip $tip > > # ipv6 packet should be received by lp[123]3 with mac > f0:00:00:00:0${i}:${i}3 > # and ip6.dst as fe80::ea2a:eaff:fe28:0${i}${i}3. > @@ -22213,7 +22213,7 @@ for mtu in 100 500 114; do > OVS_WAIT_FOR_OUTPUT([ > as hv1 ovs-ofctl dump-flows br-int > br-int-flows-$mtu > AT_CAPTURE_FILE([br-int-flows-$mtu]) > - grep "check_pkt_larger($(expr $mtu + 14))" br-int-flows-$mtu | wc > -l], [0], [4 > + grep "check_pkt_larger($(expr $mtu + 14))" br-int-flows-$mtu | wc > -l], [0], [3 > ]) > > AS_BOX([testing outgoing traffic mtu $mtu - IPv4]) > @@ -22231,7 +22231,7 @@ AT_CAPTURE_FILE([ext-sbflows-100]) > OVS_WAIT_FOR_OUTPUT([ > as hv1 ovs-ofctl dump-flows br-int > ext-br-int-flows-100 > AT_CAPTURE_FILE([ext-br-int-flows-100]) > - grep "check_pkt_larger(114)" ext-br-int-flows-100 | wc -l], [0], [4 > + grep "check_pkt_larger(114)" ext-br-int-flows-100 | wc -l], [0], [3 > ]) > > AS_BOX([testing ingress traffic mtu 100 - IPv4]) > @@ -22277,7 +22277,7 @@ for mtu in 100 500 114; do > OVS_WAIT_FOR_OUTPUT([ > as hv1 ovs-ofctl dump-flows br-int > br-int-gw-flows-$mtu > AT_CAPTURE_FILE([br-int-gw-flows-$mtu]) > - grep "check_pkt_larger($(expr $mtu + 14))" br-int-gw-flows-$mtu | > wc -l], [0], [3 > + grep "check_pkt_larger($(expr $mtu + 14))" br-int-gw-flows-$mtu | > wc -l], [0], [2 > ]) > > AS_BOX([testing outgoing traffic mtu $mtu for gw router - IPv4]) > @@ -22295,7 +22295,7 @@ AT_CAPTURE_FILE([ext-gw-sbflows-100]) > OVS_WAIT_FOR_OUTPUT([ > as hv1 ovs-ofctl dump-flows br-int > ext-br-int-gw-flows-100 > AT_CAPTURE_FILE([ext-br-int-gw-flows-100]) > - grep "check_pkt_larger(114)" ext-br-int-gw-flows-100 | wc -l], [0], [3 > + grep "check_pkt_larger(114)" ext-br-int-gw-flows-100 | wc -l], [0], [2 > ]) > > AS_BOX([testing ingress traffic mtu 100 for gw router - IPv4]) > -- > 2.52.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thank you Lorenzo, I went ahead, merged this into main and backported all the way down to 24.03. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
