| Message ID | 20240125214657.734291-1-mkp@redhat.com |
|---|---|
| State | Accepted |
| Commit | 96990ea1e4a597bff3750901ede7b92412ac443e |
| Headers | show |
| Series | [ovs-dev,1/5] dp-packet: Reset offload/offsets when clearing a packet. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | fail | test: fail |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 1/25/24 22:46, Mike Pattrick wrote: > The OVN test suite identified a bug in dp_packet_ol_send_prepare() where > a BFD packet flagged as double encapsulated would trigger a seg fault. > The problem surfaced because bfd_put_packet was reusing a packet > allocated on the stack that wasn't having its flags reset between calls. > > This change will reset OL flags as well as the layer offsets in > data_clear(), which should fix this type of packet reuse issue in > general as long as data_clear() is called in between uses. > > Fixes: 8b5fe2dc6080 ("userspace: Add Generic Segmentation Offloading.") > Reported-by: Dumitru Ceara <dceara@redhat.com> > Reported-at: https://issues.redhat.com/browse/FDP-300 > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- > lib/dp-packet.h | 3 +++ > lib/packets.c | 3 --- > 2 files changed, 3 insertions(+), 3 deletions(-) Recheck-request: github-robot
On 1/25/24 22:46, Mike Pattrick wrote: > The OVN test suite identified a bug in dp_packet_ol_send_prepare() where > a BFD packet flagged as double encapsulated would trigger a seg fault. > The problem surfaced because bfd_put_packet was reusing a packet > allocated on the stack that wasn't having its flags reset between calls. > > This change will reset OL flags as well as the layer offsets in > data_clear(), which should fix this type of packet reuse issue in > general as long as data_clear() is called in between uses. > > Fixes: 8b5fe2dc6080 ("userspace: Add Generic Segmentation Offloading.") > Reported-by: Dumitru Ceara <dceara@redhat.com> > Reported-at: https://issues.redhat.com/browse/FDP-300 > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- Thanks, Mike! This patch seems correct to me and it fixes the issue with OVN in my testing. Applied this one and backported to 3.3. I left some comments for the other patches in the set. FWIW, Dumitru, you may want to wait for the following change to land before upgrading submodule in OVN: https://patchwork.ozlabs.org/project/openvswitch/patch/20240126170758.114302-1-i.maximets@ovn.org/ Best regards, Ilya Maximets.
diff --git a/lib/dp-packet.h b/lib/dp-packet.h index 939bec5c8..dceb701e8 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -207,6 +207,7 @@ void *dp_packet_resize_l2(struct dp_packet *, int increment); void *dp_packet_resize_l2_5(struct dp_packet *, int increment); static inline void *dp_packet_eth(const struct dp_packet *); static inline void dp_packet_reset_offsets(struct dp_packet *); +static inline void dp_packet_reset_offload(struct dp_packet *); static inline uint16_t dp_packet_l2_pad_size(const struct dp_packet *); static inline void dp_packet_set_l2_pad_size(struct dp_packet *, uint16_t); static inline void *dp_packet_l2_5(const struct dp_packet *); @@ -380,6 +381,8 @@ dp_packet_clear(struct dp_packet *b) { dp_packet_set_data(b, dp_packet_base(b)); dp_packet_set_size(b, 0); + dp_packet_reset_offsets(b); + dp_packet_reset_offload(b); } /* Removes 'size' bytes from the head end of 'b', which must contain at least diff --git a/lib/packets.c b/lib/packets.c index f23d25420..36c6692e5 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -224,7 +224,6 @@ compose_rarp(struct dp_packet *b, const struct eth_addr eth_src) arp->ar_tha = eth_src; put_16aligned_be32(&arp->ar_tpa, htonl(0)); - dp_packet_reset_offsets(b); dp_packet_set_l3(b, arp); b->packet_type = htonl(PT_ETH); } @@ -1114,7 +1113,6 @@ eth_compose(struct dp_packet *b, const struct eth_addr eth_dst, eth->eth_type = htons(eth_type); b->packet_type = htonl(PT_ETH); - dp_packet_reset_offsets(b); dp_packet_set_l3(b, data); return data; @@ -1747,7 +1745,6 @@ compose_arp__(struct dp_packet *b) arp->ar_hln = sizeof arp->ar_sha; arp->ar_pln = sizeof arp->ar_spa; - dp_packet_reset_offsets(b); dp_packet_set_l3(b, arp); b->packet_type = htonl(PT_ETH);
The OVN test suite identified a bug in dp_packet_ol_send_prepare() where a BFD packet flagged as double encapsulated would trigger a seg fault. The problem surfaced because bfd_put_packet was reusing a packet allocated on the stack that wasn't having its flags reset between calls. This change will reset OL flags as well as the layer offsets in data_clear(), which should fix this type of packet reuse issue in general as long as data_clear() is called in between uses. Fixes: 8b5fe2dc6080 ("userspace: Add Generic Segmentation Offloading.") Reported-by: Dumitru Ceara <dceara@redhat.com> Reported-at: https://issues.redhat.com/browse/FDP-300 Signed-off-by: Mike Pattrick <mkp@redhat.com> --- lib/dp-packet.h | 3 +++ lib/packets.c | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-)