diff mbox series

[ovs-dev,ovn] northd: set packet length in check_pkt_larger()

Message ID 35b6cdf4751c4ac5aebd2f018c859b794997970f.1592387613.git.lorenzo.bianconi@redhat.com
State New
Headers show
Series [ovs-dev,ovn] northd: set packet length in check_pkt_larger() | expand

Commit Message

Lorenzo Bianconi June 17, 2020, 9:55 a.m. UTC
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@redhat.com>
---
 northd/ovn-northd.c |  4 ++--
 tests/ovn.at        | 33 ++++++++++++++++++---------------
 2 files changed, 20 insertions(+), 17 deletions(-)

Comments

Numan Siddique June 17, 2020, 9:58 a.m. UTC | #1
On Wed, Jun 17, 2020 at 3:26 PM Lorenzo Bianconi <
lorenzo.bianconi@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@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.

What would be the case if there is a VLAN tag set ?

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@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Lorenzo Bianconi June 17, 2020, 11:08 a.m. UTC | #2
> lorenzo.bianconi@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@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@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> >
Gabriele Cerami June 18, 2020, 8:29 a.m. UTC | #3
On 17 Jun, Numan Siddique wrote:
> On Wed, Jun 17, 2020 at 3:26 PM Lorenzo Bianconi <
> lorenzo.bianconi@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@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.
> 

Maybe it's also worth modifying the comment here
https://github.com/ovn-org/ovn/blob/master/northd/ovn-northd.c#L10103
Numan Siddique June 18, 2020, 8:33 a.m. UTC | #4
On Thu, Jun 18, 2020 at 2:00 PM Gabriele Cerami <gcerami@redhat.com> wrote:

> On 17 Jun, Numan Siddique wrote:
> > On Wed, Jun 17, 2020 at 3:26 PM Lorenzo Bianconi <
> > lorenzo.bianconi@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@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.
> >
>
> Maybe it's also worth modifying the comment here
> https://github.com/ovn-org/ovn/blob/master/northd/ovn-northd.c#L10103


+1. Thanks

Numan


>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Gabriele Cerami June 18, 2020, 8:46 a.m. UTC | #5
On 17 Jun, Lorenzo Bianconi wrote:
> > 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

I don't have the full picture of what happens either, but it would seem
the vlan header is accounted

From here:
https://github.com/torvalds/linux/blob/master/include/linux/if_vlan.h#L342
skb_push resizes the skb adding VLAN_HLEN bytes, modifying skb->len
Gabriele Cerami June 18, 2020, 9:36 a.m. UTC | #6
On 18 Jun, Gabriele Cerami wrote:
> On 17 Jun, Lorenzo Bianconi wrote:
> > > 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
> 
> I don't have the full picture of what happens either, but it would seem
> the vlan header is accounted

Nevermind, there's really a lot more into it, indeed I don't have the
full picture of the flow.
Ilya Maximets June 18, 2020, 10:12 a.m. UTC | #7
On 6/17/20 1:08 PM, Lorenzo Bianconi wrote:
>> lorenzo.bianconi@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@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

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
>>> dev@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Lorenzo Bianconi June 18, 2020, 11:59 a.m. UTC | #8
> On 6/17/20 1:08 PM, Lorenzo Bianconi wrote:
> >> lorenzo.bianconi@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@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
> 
> Anyway, 18 seems to be an ETH_HEADER_LEN (14) + VLAN_HEADER_LEN (4).  Isn't it?

Nope, I guess 18 is ETH_HEADER_LEN(14) + FCS(4)

> I don't know how this supposed to work with double tagged packets, though.

Looking at the code it seems even the inner vlan tag is not accounted in
skb->len (parse_vlan in net/openvswitch/flow.c) but I can be wrong since I am
not so familiar with this code.
Anyway I guess this would be true even with the previous implementation since
check_pkt_larger() will check packet length and not the MTU.
This patch is actually an improvement in order to not consider the L2 header in
the MTU (since the field is called "gateway_mtu")
Agree?

Regards,
Lorenzo

> 
> > 
> > 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@openvswitch.org
> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>>
> >>>
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > 
>
diff mbox series

Patch

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);
             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
 ])