Dumitru Ceara <[email protected]> writes:
> 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?
I think it makes sense. These are two different issues, so probably
should go as two separate patches.
>>
>> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
