diff mbox

[(maint?),1/3] batamn-adv: fix elp packet data reservation

Message ID 20160821032534.11074-1-linus.luessing@c0d3.blue
State Superseded, archived
Delegated to: Marek Lindner
Headers

Commit Message

Linus Lüssing Aug. 21, 2016, 3:25 a.m. UTC 
  The skb_reserve() call only reserved headroom for the mac header, but
not the elp packet header itself.

Fixing this by using skb_put()'ing towards the skb tail instead of
skb_push()'ing towards the skb head.

Fixes: a4b88af77e28 ("batman-adv: ELP - adding basic infrastructure")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

---

Looking at some tests today, we might have been lucky so far:
dev_alloc_skb(size = 32) seems to actually round the head- and tailroom
reservation to 64 bytes. So there actually is enough headroom for the
skb_push(). Not sure whether this is always the case though, so
unsure whether this should go to maint/stable.

Also switching skb_push() with skb_pull() instead of simply increasing
skb_reserve() by ELP_HLEN, because of the next patch in this series.
---
 net/batman-adv/bat_v_elp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Sven Eckelmann Aug. 21, 2016, 6:47 a.m. UTC | #1 
On Sonntag, 21. August 2016 05:25:32 CEST Linus Lüssing wrote:
[...]
> @@ -334,8 +333,9 @@ int batadv_v_elp_iface_enable(struct batadv_hard_iface *hard_iface)
>  		goto out;
>  
>  	skb_reserve(hard_iface->bat_v.elp_skb, ETH_HLEN + NET_IP_ALIGN);
> -	elp_buff = skb_push(hard_iface->bat_v.elp_skb, BATADV_ELP_HLEN);
> -	elp_packet = (struct batadv_elp_packet *)elp_buff;
> +	skb_put(hard_iface->bat_v.elp_skb, BATADV_ELP_HLEN);
> +	elp_packet = (struct batadv_elp_packet *)
> +			hard_iface->bat_v.elp_skb->data;
>  	memset(elp_packet, 0, BATADV_ELP_HLEN);
>  
>  	elp_packet->packet_type = BATADV_ELP;
> 

I don't get right now why you did the of split the skb_put into two different
"ugly" lines (skb_put + the elp_packet assignment without elp_buff). I fear 
that this weird (non)-alignment you've created will bite us when the patch is 
submitted upstream.

Maybe you can tell us more about why this removal of elp_buff is necessary.

Btw. please use the prefix "batman-adv" in the subject and not "batamn-adv" ;)

And it is at the moment not important whether it goes into next or
maint. Both will be submitted by Simon to net.git because we are currently
completely off with our timing (compared to the upstream submissions).
I personally would go for maint.

Kind regards,
	Sven
diff mbox

Patch

diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index df42eb1..ea463bf 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -323,7 +323,6 @@  out:
 int batadv_v_elp_iface_enable(struct batadv_hard_iface *hard_iface)
 {
 	struct batadv_elp_packet *elp_packet;
-	unsigned char *elp_buff;
 	u32 random_seqno;
 	size_t size;
 	int res = -ENOMEM;
@@ -334,8 +333,9 @@  int batadv_v_elp_iface_enable(struct batadv_hard_iface *hard_iface)
 		goto out;
 
 	skb_reserve(hard_iface->bat_v.elp_skb, ETH_HLEN + NET_IP_ALIGN);
-	elp_buff = skb_push(hard_iface->bat_v.elp_skb, BATADV_ELP_HLEN);
-	elp_packet = (struct batadv_elp_packet *)elp_buff;
+	skb_put(hard_iface->bat_v.elp_skb, BATADV_ELP_HLEN);
+	elp_packet = (struct batadv_elp_packet *)
+			hard_iface->bat_v.elp_skb->data;
 	memset(elp_packet, 0, BATADV_ELP_HLEN);
 
 	elp_packet->packet_type = BATADV_ELP;