All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Wunderlich <sw@simonwunderlich.de>
To: kuba@kernel.org, davem@davemloft.net
Cc: netdev@vger.kernel.org, b.a.t.m.a.n@lists.open-mesh.org,
	"Linus Lüssing" <linus.luessing@c0d3.blue>,
	"Sven Eckelmann" <sven@narfation.org>,
	"Simon Wunderlich" <sw@simonwunderlich.de>
Subject: [PATCH 6/6] batman-adv: bcast: remove remaining skb-copy calls
Date: Fri, 20 Aug 2021 10:33:00 +0200	[thread overview]
Message-ID: <20210820083300.32289-7-sw@simonwunderlich.de> (raw)
In-Reply-To: <20210820083300.32289-1-sw@simonwunderlich.de>

From: Linus Lüssing <linus.luessing@c0d3.blue>

We currently have two code paths for broadcast packets:

A) self-generated, via batadv_interface_tx()->
   batadv_send_bcast_packet().
B) received/forwarded, via batadv_recv_bcast_packet()->
   batadv_forw_bcast_packet().

For A), self-generated broadcast packets:

The only modifications to the skb data is the ethernet header which is
added/pushed to the skb in
batadv_send_broadcast_skb()->batadv_send_skb_packet(). However before
doing so, batadv_skb_head_push() is called which calls skb_cow_head() to
unshare the space for the to be pushed ethernet header. So for this
case, it is safe to use skb clones.

For B), received/forwarded packets:

The same applies as in A) for the to be forwarded packets. Only the
ethernet header is added. However after (queueing for) forwarding the
packet in batadv_recv_bcast_packet()->batadv_forw_bcast_packet(), a
packet is additionally decapsulated and is sent up the stack through
batadv_recv_bcast_packet()->batadv_interface_rx().

Protocols higher up the stack are already required to check if the
packet is shared and create a copy for further modifications. When the
next (protocol) layer works correctly, it cannot happen that it tries to
operate on the data behind the skb clone which is still queued up for
forwarding.

Co-authored-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/send.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index 2a33458be65c..477d85a3b558 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -742,6 +742,10 @@ void batadv_forw_packet_ogmv1_queue(struct batadv_priv *bat_priv,
  * Adds a broadcast packet to the queue and sets up timers. Broadcast packets
  * are sent multiple times to increase probability for being received.
  *
+ * This call clones the given skb, hence the caller needs to take into
+ * account that the data segment of the original skb might not be
+ * modifiable anymore.
+ *
  * Return: NETDEV_TX_OK on success and NETDEV_TX_BUSY on errors.
  */
 static int batadv_forw_bcast_packet_to_list(struct batadv_priv *bat_priv,
@@ -755,7 +759,7 @@ static int batadv_forw_bcast_packet_to_list(struct batadv_priv *bat_priv,
 	unsigned long send_time = jiffies;
 	struct sk_buff *newskb;
 
-	newskb = skb_copy(skb, GFP_ATOMIC);
+	newskb = skb_clone(skb, GFP_ATOMIC);
 	if (!newskb)
 		goto err;
 
@@ -794,6 +798,10 @@ static int batadv_forw_bcast_packet_to_list(struct batadv_priv *bat_priv,
  * or if a delay is given after that. Furthermore, queues additional
  * retransmissions if this interface is a wireless one.
  *
+ * This call clones the given skb, hence the caller needs to take into
+ * account that the data segment of the original skb might not be
+ * modifiable anymore.
+ *
  * Return: NETDEV_TX_OK on success and NETDEV_TX_BUSY on errors.
  */
 static int batadv_forw_bcast_packet_if(struct batadv_priv *bat_priv,
@@ -808,7 +816,7 @@ static int batadv_forw_bcast_packet_if(struct batadv_priv *bat_priv,
 	int ret = NETDEV_TX_OK;
 
 	if (!delay) {
-		newskb = skb_copy(skb, GFP_ATOMIC);
+		newskb = skb_clone(skb, GFP_ATOMIC);
 		if (!newskb)
 			return NETDEV_TX_BUSY;
 
-- 
2.20.1


  parent reply	other threads:[~2021-08-20  8:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20  8:32 [PATCH 0/6] (updated) pull request for net-next: batman-adv 2021-08-20 Simon Wunderlich
2021-08-20  8:32 ` Simon Wunderlich
2021-08-20  8:32 ` [PATCH 1/6] batman-adv: Start new development cycle Simon Wunderlich
2021-08-20  8:32   ` Simon Wunderlich
2021-08-20 12:52   ` patchwork-bot+netdevbpf
2021-08-20  8:32 ` [PATCH 2/6] batman-adv: Move IRC channel to hackint.org Simon Wunderlich
2021-08-20  8:32 ` [PATCH 3/6] batman-adv: Switch to kstrtox.h for kstrtou64 Simon Wunderlich
2021-08-20  8:32 ` [PATCH 4/6] batman-adv: Check ptr for NULL before reducing its refcnt Simon Wunderlich
2021-08-20  8:32 ` [PATCH 5/6] batman-adv: Drop NULL check before dropping references Simon Wunderlich
2021-08-20  8:33 ` Simon Wunderlich [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-08-19 15:33 [PATCH 0/6] pull request for net-next: batman-adv 2021-08-19 Simon Wunderlich
2021-08-19 15:33 ` [PATCH 6/6] batman-adv: bcast: remove remaining skb-copy calls Simon Wunderlich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210820083300.32289-7-sw@simonwunderlich.de \
    --to=sw@simonwunderlich.de \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linus.luessing@c0d3.blue \
    --cc=netdev@vger.kernel.org \
    --cc=sven@narfation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.