On Wed, Feb 26, 2025 at 5:10 AM Mike Pattrick <m...@redhat.com> wrote:
> > > @@ -961,10 +961,10 @@ netdev_pop_header(struct netdev *netdev, struct
> > > dp_packet_batch *batch)
> > > DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, batch) {
> > > packet = netdev->netdev_class->pop_header(packet);
> > > if (packet) {
> > > - /* Reset the offload flags if present, to avoid wrong
> > > + /* Reset the TX offload flags if present, to avoid wrong
> > > * interpretation in the further packet processing when
> > > * recirculated.*/
> > > - dp_packet_reset_offload(packet);
> > > + dp_packet_reset_tx_offload(packet);
> >
> > I am not following here.
> >
> > With the current code, when popping a header (of a tunnel packet
> > received from a port) and resetting those Rx flags, in case of
> > conntrack, the packet will be L4 "unknown" and the csum will be
> > checked for the inner part.
>
> Currently the checksum will almost always be checked.
>
> This sort of works because packets from external won't have a partial
> checksum. Encapsulated checksum offloaded virtio packets don't work
> due to FDP-1147, and netdev-linux packets are saved by csum_start /
> csum_offset, which had the byproduct of also tagging the packet as
> partial after the flags are reset.
>
> > While with this change, won't an invalid L4 packet encapsulated in a
> > valid udp header will be treated as valid L4?
>
> This is a good point. How do you feel about carrying over partial
> flag, but not other flags?
That should work.
>
> >
> > I also wonder: is it possible that an L4 invalid (non tunneled) packet
> > enters OVS, then gets encapsulated (here, L4 inner csum status gets
> > lost), then after some packet processing gets decapsulated again (and
> > then fixed on the egress path)?
> >
> > I also have some doubt about receiving a eth / ip / udp / vxlan / eth
> > / arp packet.
> > On popping the header, we would keep the Rx L4 info for the UDP
> > header, on a arp packet.
>
> But aren't the RX flags only used in conjunction with TX flags, conn key, etc?
Well, that's probably where I am not happy.
Having the Rx and Tx intermixed is complex.
I tracked inner / outer status in my PoC, and I think that simplifies
the API a lot.
Let me share this code and we can decide what would be better.
--
David Marchand
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
