All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] af-packet: new flag to indicate all csums are good
@ 2020-06-01 20:49 Victor Julien
  2020-06-01 21:45 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Victor Julien @ 2020-06-01 20:49 UTC (permalink / raw)
  To: netdev
  Cc: victor, David S. Miller, Jakub Kicinski, Jonathan Corbet,
	Eric Dumazet, Willem de Bruijn, Mao Wenan, Arnd Bergmann,
	Neil Horman, linux-doc, linux-kernel

Introduce a new flag (TP_STATUS_CSUM_UNNECESSARY) to indicate
that the driver has completely validated the checksums in the packet.

The flag differs from TP_STATUS_CSUM_VALID in that it 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.

The name is derived from the skb->ip_summed setting CHECKSUM_UNNECESSARY.

Security tools such as Suricata, Snort, Zeek/Bro need to know not
only that a packet has not been corrupted, but also that the
checksums are correct. Without this an attacker could send a packet,
for example a TCP RST packet, that would be accepted by the
security tool, but rejected by the end host.

To avoid this scenario tools will have to (re)calcultate/validate
the checksum as well.

This patch has been tested with Suricata with the virtio driver,
where it reduced the ammount of time spent in the Suricata TCP
checksum validation to about half.

Signed-off-by: Victor Julien <victor@inliniac.net>
---
 Documentation/networking/packet_mmap.rst | 6 ++++++
 include/uapi/linux/if_packet.h           | 1 +
 net/packet/af_packet.c                   | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/Documentation/networking/packet_mmap.rst b/Documentation/networking/packet_mmap.rst
index 6c009ceb1183..f670292e6d95 100644
--- a/Documentation/networking/packet_mmap.rst
+++ b/Documentation/networking/packet_mmap.rst
@@ -472,6 +472,12 @@ TP_STATUS_CSUM_VALID	This flag indicates that at least the transport
 			validated on the kernel side. If the flag is not set
 			then we are free to check the checksum by ourselves
 			provided that TP_STATUS_CSUMNOTREADY is also not set.
+TP_STATUS_CSUM_UNNECESSARY  This flag indicates that the driver validated all
+                        the packets csums. If it is not set it might be that
+                        the driver doesn't support this, or that one of the
+                        layers csums is bad. TP_STATUS_CSUM_VALID may still
+                        be set if the transport layer csum is correct or
+                        if the driver supports only this mode.
 ======================  =======================================================
 
 for convenience there are also the following defines::
diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_packet.h
index 3d884d68eb30..76a5c762e2e0 100644
--- a/include/uapi/linux/if_packet.h
+++ b/include/uapi/linux/if_packet.h
@@ -113,6 +113,7 @@ struct tpacket_auxdata {
 #define TP_STATUS_BLK_TMO		(1 << 5)
 #define TP_STATUS_VLAN_TPID_VALID	(1 << 6) /* auxdata has valid tp_vlan_tpid */
 #define TP_STATUS_CSUM_VALID		(1 << 7)
+#define TP_STATUS_CSUM_UNNECESSARY	(1 << 8)
 
 /* Tx ring - header status */
 #define TP_STATUS_AVAILABLE	      0
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 29bd405adbbd..5dd8bad9bc23 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2215,6 +2215,9 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL)
 		status |= TP_STATUS_CSUMNOTREADY;
+	else if (skb->pkt_type != PACKET_OUTGOING &&
+		 skb->ip_summed == CHECKSUM_UNNECESSARY)
+		status |= (TP_STATUS_CSUM_UNNECESSARY | TP_STATUS_CSUM_VALID);
 	else if (skb->pkt_type != PACKET_OUTGOING &&
 		 (skb->ip_summed == CHECKSUM_COMPLETE ||
 		  skb_csum_unnecessary(skb)))
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] af-packet: new flag to indicate all csums are good
  2020-06-01 20:49 [PATCH net-next] af-packet: new flag to indicate all csums are good Victor Julien
@ 2020-06-01 21:45 ` David Miller
  2020-06-02  4:23   ` Victor Julien
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2020-06-01 21:45 UTC (permalink / raw)
  To: victor
  Cc: netdev, kuba, corbet, edumazet, willemb, maowenan, arnd, nhorman,
	linux-doc, linux-kernel

From: Victor Julien <victor@inliniac.net>
Date: Mon,  1 Jun 2020 22:49:37 +0200

> @@ -472,6 +472,12 @@ TP_STATUS_CSUM_VALID	This flag indicates that at least the transport
>  			validated on the kernel side. If the flag is not set
>  			then we are free to check the checksum by ourselves
>  			provided that TP_STATUS_CSUMNOTREADY is also not set.
> +TP_STATUS_CSUM_UNNECESSARY  This flag indicates that the driver validated all
> +                        the packets csums. If it is not set it might be that
> +                        the driver doesn't support this, or that one of the
> +                        layers csums is bad. TP_STATUS_CSUM_VALID may still
> +                        be set if the transport layer csum is correct or
> +                        if the driver supports only this mode.
>  ======================  =======================================================
                        ^^^^^

I think you need to reformat these dividers.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next] af-packet: new flag to indicate all csums are good
  2020-06-01 21:45 ` David Miller
@ 2020-06-02  4:23   ` Victor Julien
  0 siblings, 0 replies; 3+ messages in thread
From: Victor Julien @ 2020-06-02  4:23 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, kuba, corbet, edumazet, willemb, maowenan, arnd, nhorman,
	linux-doc, linux-kernel

On 01-06-2020 23:45, David Miller wrote:
> From: Victor Julien <victor@inliniac.net>
> Date: Mon,  1 Jun 2020 22:49:37 +0200
> 
>> @@ -472,6 +472,12 @@ TP_STATUS_CSUM_VALID	This flag indicates that at least the transport
>>  			validated on the kernel side. If the flag is not set
>>  			then we are free to check the checksum by ourselves
>>  			provided that TP_STATUS_CSUMNOTREADY is also not set.
>> +TP_STATUS_CSUM_UNNECESSARY  This flag indicates that the driver validated all
>> +                        the packets csums. If it is not set it might be that
>> +                        the driver doesn't support this, or that one of the
>> +                        layers csums is bad. TP_STATUS_CSUM_VALID may still
>> +                        be set if the transport layer csum is correct or
>> +                        if the driver supports only this mode.
>>  ======================  =======================================================
>                         ^^^^^
> 
> I think you need to reformat these dividers.
> 

Yes of course. Think I'll have to reformat the whole table then, at
least for `rst2pdf` to accept it.

Alternatively I can try to come up with a shorter name for the flag, but
I'm not really coming up with anything so far.

-- 
---------------------------------------------
Victor Julien
http://www.inliniac.net/
PGP: http://www.inliniac.net/victorjulien.asc
---------------------------------------------


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-06-02  4:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-01 20:49 [PATCH net-next] af-packet: new flag to indicate all csums are good Victor Julien
2020-06-01 21:45 ` David Miller
2020-06-02  4:23   ` Victor Julien

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.