On 1/22/24 21:33, Mike Pattrick wrote:
> On Mon, Jan 22, 2024 at 1:47 PM Ilya Maximets <[email protected]> wrote:
>>
>> On 1/22/24 18:51, Mike Pattrick wrote:
>>> The OVN test suite identified a bug in dp_packet_ol_send_prepare() where
>>> a double encapsulated BFD packet would trigger a seg fault. This
>>> happened because we had assumed that if IP checksumming was marked as
>>> offloaded, that checksum would occur in the innermost packet.
>>>
>>> This change will check that the inner packet has an L3 and L4 before
>>> checksumming those parts. And if missing, will resume checksumming the
>>> outer components.
>>
>> Hrm. This looks like a workaround rather than a fix. If the inner packet
>> doesn't have L3 header, dp-packet must not have a flag for L3 checksumming
>> set in the first place. And if it does have inner L3, the offset must be
>> initialized. I guess, some of the offsets can be not initialized, because
>> the packet is never parsed by either miniflow_extract() or parse_tcp_flags().
>> And the bfd_put_packet() doesn't seem to set them.
>
> I think you're right, I stepped through the problem in GDB and it was
> clear that the flags weren't getting reset properly between BFD
> packets. I'll send an updated patch.
Yeah, flags preservation is one thing, the other is that l3/l4_ofs are just
not populated in these packets, which doesn't sound right.
It's also not clear why the packet is marked for inner checksum offload if
the inner checksum is actually fully calculated and correct. I understand
that the flags are carried over from a previous packet, but why did that
previous packet have this flag set?
>
>> BTW, is there actually a double encapsulation in the original OVN test?
>> Sounds strange.
>
> The problem was exposed in netdev_push_header() in
> dp_packet_ol_send_prepare(packet, 0);
That's true, but the packet dump in gdb showed a plain BFD packet.
So, it was a first encapsulation, not double.
>
> -M
>
>>
>>>
>>> Fixes: 8b5fe2dc6080 ("userspace: Add Generic Segmentation Offloading.")
>>> Reported-by: Dumitru Ceara <[email protected]>
>>> Reported-at: https://issues.redhat.com/browse/FDP-300
>>> Signed-off-by: Mike Pattrick <[email protected]>
>>> ---
>>> lib/dp-packet.h | 10 ++++++++--
>>> lib/packets.c | 6 +++---
>>> 2 files changed, 11 insertions(+), 5 deletions(-)
>>
>> Best regards, Ilya Maximets.
>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
