We currently have two code paths for broadcast packets: A) self-generated, via batadv_interface_tx()->batadv_send_bcast_packet(). Or 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(). Which needs an unshared skb data for potential modifications from other protocols. To be able to use skb clones for (re)broadcasted batman-adv broadcast packets while still ensuring that interface_rx() has a freely writeable skb we use skb_cow() to avoid sharing skb data with the skb clones in the forwarding calls.
Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue ---
Changelog v3:
* newly added this patch, to move skb_copy()->skb_clone() changes from PATCH 01/03 to a separate patch with its own explanation
net/batman-adv/send.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c index 0b9dd29d..1db6b217 100644 --- a/net/batman-adv/send.c +++ b/net/batman-adv/send.c @@ -748,6 +748,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, @@ -761,7 +765,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;
@@ -800,6 +804,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, @@ -814,7 +822,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;
@@ -966,6 +974,8 @@ static int __batadv_forw_bcast_packet(struct batadv_priv *bat_priv, * after that. Furthermore, queues additional retransmissions on wireless * interfaces. * + * This call might reallocate skb data. + * * Return: NETDEV_TX_OK on success and NETDEV_TX_BUSY on errors. */ int batadv_forw_bcast_packet(struct batadv_priv *bat_priv, @@ -973,7 +983,15 @@ int batadv_forw_bcast_packet(struct batadv_priv *bat_priv, unsigned long delay, bool own_packet) { - return __batadv_forw_bcast_packet(bat_priv, skb, delay, own_packet); + int ret = __batadv_forw_bcast_packet(bat_priv, skb, delay, own_packet); + + if (ret == NETDEV_TX_BUSY) + return ret; + + /* __batadv_forw_bcast_packet clones, make sure original + * skb stays writeable + */ + return (skb_cow(skb, 0) < 0) ? NETDEV_TX_BUSY : NETDEV_TX_OK; }
/**