On 1/23/24 00:11, 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.
>
Thanks for tracking this one down, Mike!
> This change will reset OL flags in data_clear(), which should fix this
> type of packet reuse issue in general as long as data_clear() is called
> in between uses. This change also includes a tangentially related check
> in dp_packet_inner_l4_size(), where the correct offset was not being
> checked.
Up to maintainers but should this tangential fix be a separate patch?
>
> Fixes: 8b5fe2dc6080 ("userspace: Add Generic Segmentation Offloading.")
> Fixes: 85bcbbed839a ("userspace: Enable tunnel tests with TSO.")
> Reported-by: Dumitru Ceara <[email protected]>
> Reported-at: https://issues.redhat.com/browse/FDP-300
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
I'm no expert in this code but the change itself looks correct to me and
OVN tests pass with this applied:
Reviewed-by: Dumitru Ceara <[email protected]>
> lib/dp-packet.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 939bec5c8..f328a6637 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,7 @@ 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_offload(b);
> }
>
> /* Removes 'size' bytes from the head end of 'b', which must contain at least
> @@ -537,7 +539,7 @@ dp_packet_inner_l4(const struct dp_packet *b)
> static inline size_t
> dp_packet_inner_l4_size(const struct dp_packet *b)
> {
> - return OVS_LIKELY(b->l4_ofs != UINT16_MAX)
> + return OVS_LIKELY(b->inner_l4_ofs != UINT16_MAX)
> ? (const char *) dp_packet_tail(b)
> - (const char *) dp_packet_inner_l4(b)
> - dp_packet_l2_pad_size(b)
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
