[ovs-dev] [PATCH ovn v2] pinctrl: Fix icmp6 packet corruption issue

Numan Siddique numans at ovn.org
Tue May 12 09:57:06 UTC 2020


On Tue, May 12, 2020 at 3:19 PM Dumitru Ceara <dceara at redhat.com> wrote:

> On 5/12/20 11:43 AM, numans at ovn.org wrote:
> > From: Numan Siddique <numans at ovn.org>
> >
> > The commit f792b1a00b43("Fix ACL reject action for UDP packets.")
> > didn't updated the 'struct ip6_hdr' pointer after calling
> > dp_packet_put(), as dp_packet_put() can reallocate memory making the
> > old references to packet pointers invalid.
> >
> > This patch fixes this issue.
> >
> > Fixes: f792b1a00b43("Fix ACL reject action for UDP packets.")
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1834655
> > Signed-off-by: Numan Siddique <numans at ovn.org>
> > ---
> > v1 -> v2
> > ---
> >  * Changed the contents of file foo to a better one - Suggested by
> Dumitru.
> >
> >  controller/pinctrl.c | 4 ++--
> >  tests/system-ovn.at  | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > index 6b0ac3483..d976ec82b 100644
> > --- a/controller/pinctrl.c
> > +++ b/controller/pinctrl.c
> > @@ -1570,8 +1570,6 @@ pinctrl_handle_icmp(struct rconn *swconn, const
> struct flow *ip_flow,
> >          }
> >          ih->icmp6_base.icmp6_cksum = 0;
> >
> > -        nh = dp_packet_l3(&packet);
> > -
> >          /* RFC 4443: 3.1.
> >           *
> >           * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9
> 0 1
> > @@ -1594,9 +1592,11 @@ pinctrl_handle_icmp(struct rconn *swconn, const
> struct flow *ip_flow,
> >          }
> >
> >          dp_packet_put(&packet, in_ip, in_ip_len);
> > +        nh = dp_packet_l3(&packet);
> >          nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN + in_ip_len);
> >
> >          icmpv6_csum = packet_csum_pseudoheader6(dp_packet_l3(&packet));
> > +        ih = dp_packet_l4(&packet);
> >          ih->icmp6_base.icmp6_cksum = csum_finish(
> >              csum_continue(icmpv6_csum, ih,
> >                            in_ip_len + ICMP6_DATA_HEADER_LEN));
> > diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> > index 9a5ef1ec3..aa0578573 100644
> > --- a/tests/system-ovn.at
> > +++ b/tests/system-ovn.at
> > @@ -3967,7 +3967,7 @@ OVS_WAIT_UNTIL([
> >  NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 90
> > sw0-p1-rej-udp.pcap &], [0])
> >  NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp >
> sw0-p1-rej-icmp.pcap &], [0])
> >
> > -echo "foo" > foo
> > +echo ""printf '.%.0s' {1..100}"" > foo
>
> This is fine but this would be better, I think :)
>
> printf '.%.0s' {1..100} > foo
>
> With that:
> Acked-by: Dumitru Ceara <dceara at redhat.com>
>

Ack. I modified as you suggested and applied the patch to master and
branch-20.03.

Thanks a lot for quick review.

Numan


>
> Thanks,
> Dumitru
>
> >  OVS_WAIT_UNTIL([
> >      ip netns exec sw0-p1-rej nc -u 10.0.0.4 90 < foo
> >      c=$(cat sw0-p1-rej-icmp.pcap | grep \
> >
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list