Messages in this thread | | | From | Willem de Bruijn <> | Date | Tue, 2 Jun 2020 13:37:01 -0400 | Subject | Re: [PATCH net-next v2] af-packet: new flag to indicate all csums are good |
| |
On Tue, Jun 2, 2020 at 1:03 PM Victor Julien <victor@inliniac.net> wrote: > > On 02-06-2020 16:29, Willem de Bruijn wrote: > > On Tue, Jun 2, 2020 at 4:05 AM Victor Julien <victor@inliniac.net> wrote: > >> > >> Introduce a new flag (TP_STATUS_CSUM_UNNECESSARY) to indicate > >> that the driver has completely validated the checksums in the packet. > >> > >> The TP_STATUS_CSUM_UNNECESSARY flag differs from TP_STATUS_CSUM_VALID > >> in that the new flag will only be set if all the layers are valid, > >> while TP_STATUS_CSUM_VALID is set as well if only the IP layer is valid. > > > > transport, not ip checksum. > > Allow me a n00b question: what does transport refer to here? Things like > ethernet? It isn't clear to me from the doc.
The TCP/UDP/.. transport protocol checksum.
> (happy to follow up with a patch to clarify the doc when I understand > things better) > > > But as I understand it drivers set CHECKSUM_COMPLETE if they fill in > > skb->csum over the full length of the packet. This does not > > necessarily imply that any of the checksum fields in the packet are > > valid yet (see also skb->csum_valid). Protocol code later compares > > checksum fields against this using __skb_checksum_validate_complete and friends. > > > > But packet sockets may be called before any of this, however. So I wonder > > how valid the checksum really is right now when setting TP_STATUS_CSUM_VALID. > > I assume it's correct, but don't fully understand where the validation > > has taken place.. > > I guess I'm more confused now about what TP_STATUS_CSUM_VALID actually > means. It sounds almost like the opposite of TP_STATUS_CSUMNOTREADY, but > I'm not sure I understand what the value would be. > > It would be great if someone could help clear this up. Everything I > thought I knew/understood so far has been proven wrong, so I'm not too > confident about my patch anymore...
Agreed that we should clear this up.
> > Similar to commit 682f048bd494 ("af_packet: pass checksum validation > > status to the user"), please update tpacket_rcv and packet_rcv. > > Ah yes, good catch. Will add it there as well. > > > Note also that net-next is currently closed. > > Should I hold off on sending a v3 until it reopens?
Yep, thanks. You can always check http://vger.kernel.org/~davem/net-next.html when unsure.
| |