[ovs-dev] [PATCH ovn] northd: set packet length in check_pkt_larger()
Lorenzo Bianconi
lorenzo.bianconi at redhat.com
Wed Jun 17 11:08:41 UTC 2020
> lorenzo.bianconi at redhat.com> 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 <lorenzo.bianconi at redhat.com>
> > ---
> > 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
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
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> >
More information about the dev
mailing list