| Message ID | 20200512092557.530039-1-numans@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,ovn] pinctrl: Fix icmp6 packet corruption issue | expand |
On 5/12/20 11:25 AM, numans@ovn.org wrote: > From: Numan Siddique <numans@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@ovn.org> Hi Numan, I think I'd change the foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo to something a bit different but except for that: Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, 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..d0240840d 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 "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" > foo > 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 \ >
On Tue, May 12, 2020 at 3:14 PM Dumitru Ceara <dceara@redhat.com> wrote: > On 5/12/20 11:25 AM, numans@ovn.org wrote: > > From: Numan Siddique <numans@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@ovn.org> > > Hi Numan, > > I think I'd change the > > foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo > to something a bit different but except for that: > > Acked-by: Dumitru Ceara <dceara@redhat.com> > I just submitted v2 wich you suggested me offline - echo ""printf '.%.0s' {1 ..100}"" > foo Thanks Numan > > Thanks, > 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..d0240840d 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 > "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" > > foo > > 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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 pinctrl: Fix icmp6 packet corruption issue When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
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..d0240840d 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 "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" > foo 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 \