On 6/17/20 1:08 PM, Lorenzo Bianconi wrote: >> [email protected]> wrote: >> >>> Set packet length in lr_in_chk_pkt_len router pipeline instead of >>> gw interface MTU since ovs kernel datapath usually works on L2 frames >>> >>> Fixes: 7d42c146be ("ovn: Generate ICMPv4 packet in router pipeline for >>> larger packets") >>> Signed-off-by: Lorenzo Bianconi <[email protected]> >>> --- >>> northd/ovn-northd.c | 4 ++-- >>> tests/ovn.at | 33 ++++++++++++++++++--------------- >>> 2 files changed, 20 insertions(+), 17 deletions(-) >>> >>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >>> index b8c9e9325..53d5bf245 100644 >>> --- a/northd/ovn-northd.c >>> +++ b/northd/ovn-northd.c >>> @@ -10076,7 +10076,7 @@ build_lrouter_flows(struct hmap *datapaths, struct >>> hmap *ports, >>> ds_clear(&actions); >>> ds_put_format(&actions, >>> REGBIT_PKT_LARGER" = check_pkt_larger(%d);" >>> - " next;", gw_mtu); >>> + " next;", gw_mtu + 18); >>> >> >> Hi Lorenzo, >> >> Thanks for the fix. Can you please add the comment why 18. May be a macro >> instead of "18" number. > > Hi Numan, > > ack, will do in v2 > >> >> What would be the case if there is a VLAN tag set ? > > I am not expert about how ovs manages vlan tags, but looking at [1] and [2], > it seems > to me vlan header is not accounted in skb->len
Anyway, 18 seems to be an ETH_HEADER_LEN (14) + VLAN_HEADER_LEN (4). Isn't it? I don't know how this supposed to work with double tagged packets, though. > > Regards, > Lorenzo > > [1] https://github.com/torvalds/linux/blob/master/net/core/dev.c#L5117 > [2] > https://github.com/torvalds/linux/blob/master/net/openvswitch/vport-netdev.c#L29 > >> >> Thanks >> Numan >> >> >> >> >>> ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_CHK_PKT_LEN, >>> 50, >>> ds_cstr(&match), ds_cstr(&actions), >>> &od->l3dgw_port->nbrp->header_); >>> @@ -10109,7 +10109,7 @@ build_lrouter_flows(struct hmap *datapaths, struct >>> hmap *ports, >>> "next(pipeline=ingress, table=0); };", >>> rp->lrp_networks.ea_s, >>> rp->lrp_networks.ipv4_addrs[0].addr_s, >>> - gw_mtu - 18); >>> + gw_mtu); >>> ovn_lflow_add_with_hint(lflows, od, >>> S_ROUTER_IN_LARGER_PKTS, >>> 50, ds_cstr(&match), >>> ds_cstr(&actions), >>> &rp->nbrp->header_); >>> diff --git a/tests/ovn.at b/tests/ovn.at >>> index 7e1ace556..1801a3151 100644 >>> --- a/tests/ovn.at >>> +++ b/tests/ovn.at >>> @@ -14801,14 +14801,16 @@ test_ip_packet_larger() { >>> dst_mac="00000000ff01" # sw0-lr0 mac (internal router leg) >>> src_ip=`ip_to_hex 10 0 0 3` >>> dst_ip=`ip_to_hex 172 168 0 3` >>> - # Set the packet length to 100. >>> - pkt_len=0064 >>> - packet=${dst_mac}${src_mac}08004500${pkt_len}0000000040010000 >>> + # Set the packet length to 118. >>> + pkt_len=0076 >>> + packet=${dst_mac}${src_mac}08004500${pkt_len}000000004001c3d9 >>> orig_packet_l3=${src_ip}${dst_ip}0304000000000000 >>> orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000 >>> orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000 >>> orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000 >>> orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000 >>> + orig_packet_l3=${orig_packet_l3}000000000000000000000000000000000000 >>> + >>> packet=${packet}${orig_packet_l3} >>> >>> >>> >>> gw_ip_garp=ffffffffffff00002020121308060001080006040001000020201213aca80064000000000000aca80064 >>> @@ -14818,32 +14820,33 @@ test_ip_packet_larger() { >>> # localnet port. >>> # If icmp_pmtu_reply_expected is 1, it means the packet is larger than >>> # the gateway mtu and ovn-controller should drop the packet and >>> instead >>> - # generate ICMPv4 Destination Unreachable message with pmtu set to >>> 42. >>> + # generate ICMPv4 Destination Unreachable message with pmtu set to >>> 100. >>> if test $icmp_pmtu_reply_expected = 0; then >>> # Packet to expect at br-phys. >>> src_mac="000020201213" >>> dst_mac="00000012af11" >>> src_ip=`ip_to_hex 10 0 0 3` >>> dst_ip=`ip_to_hex 172 168 0 3` >>> - expected=${dst_mac}${src_mac}08004500${pkt_len}000000003f010100 >>> + expected=${dst_mac}${src_mac}08004500${pkt_len}000000003f01c4d9 >>> expected=${expected}${src_ip}${dst_ip}0304000000000000 >>> expected=${expected}000000000000000000000000000000000000 >>> expected=${expected}000000000000000000000000000000000000 >>> expected=${expected}000000000000000000000000000000000000 >>> expected=${expected}000000000000000000000000000000000000 >>> + expected=${expected}000000000000000000000000000000000000 >>> echo $expected > br_phys_n1.expected >>> echo $gw_ip_garp >> br_phys_n1.expected >>> else >>> - # MTU would be 100 - 18 = 82 (hex 0052) >>> - mtu=0052 >>> + # MTU would be 118 - 18 = 100 (hex 0064) >>> + mtu=0064 >>> src_ip=`ip_to_hex 10 0 0 1` >>> dst_ip=`ip_to_hex 10 0 0 3` >>> - # pkt len should be 128 (28 (icmp packet) + 100 (orig ip + >>> payload)) >>> - reply_pkt_len=0080 >>> - ip_csum=bd91 >>> - >>> icmp_reply=${src_mac}${dst_mac}08004500${reply_pkt_len}00004000fe016879 >>> + # pkt len should be 146 (28 (icmp packet) + 118 (orig ip + >>> payload)) >>> + reply_pkt_len=0092 >>> + ip_csum=f993 >>> + >>> icmp_reply=${src_mac}${dst_mac}08004500${reply_pkt_len}00004000fe016867 >>> icmp_reply=${icmp_reply}${src_ip}${dst_ip}0304${ip_csum}0000${mtu} >>> - icmp_reply=${icmp_reply}4500${pkt_len}000000003f010100 >>> + icmp_reply=${icmp_reply}4500${pkt_len}000000003f01c4d9 >>> icmp_reply=${icmp_reply}${orig_packet_l3} >>> echo $icmp_reply > hv1-vif1.expected >>> fi >>> @@ -14883,12 +14886,12 @@ awk '{print $3}') >>> ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \ >>> logical_port=lr0-public mac="00\:00\:00\:12\:af\:11" >>> >>> -# Set the gateway mtu to 100. If the packet length is > 100, >>> ovn-controller >>> +# Set the gateway mtu to 100. If the packet length is > 118, >>> ovn-controller >>> # should send icmp host not reachable with pmtu set to 100. >>> ovn-nbctl --wait=hv set logical_router_port lr0-public >>> options:gateway_mtu=100 >>> as hv3 ovs-appctl netdev-dummy/receive hv3-vif1 $arp_reply >>> OVS_WAIT_UNTIL([ >>> - test `as hv1 ovs-ofctl dump-flows br-int | grep >>> "check_pkt_larger(100)" | \ >>> + test `as hv1 ovs-ofctl dump-flows br-int | grep >>> "check_pkt_larger(118)" | \ >>> wc -l` -eq 1 >>> ]) >>> >>> @@ -14899,7 +14902,7 @@ test_ip_packet_larger $icmp_reply_expected >>> ovn-nbctl --wait=hv set logical_router_port lr0-public >>> options:gateway_mtu=500 >>> as hv3 ovs-appctl netdev-dummy/receive hv3-vif1 $arp_reply >>> OVS_WAIT_UNTIL([ >>> - test `as hv1 ovs-ofctl dump-flows br-int | grep >>> "check_pkt_larger(500)" | \ >>> + test `as hv1 ovs-ofctl dump-flows br-int | grep >>> "check_pkt_larger(518)" | \ >>> wc -l` -eq 1 >>> ]) >>> >>> -- >>> 2.26.2 >>> >>> _______________________________________________ >>> 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
