It is possible that the mac_len is not properly exported because of strange device configuration (this behaviour has been observed when using batman-adv on top of a vlan interface). Therefore it is needed to explicitly recompute it at the very beginning of the rx path
Signed-off-by: Antonio Quartulli ordex@autistici.org --- main.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/main.c b/main.c index f9bcfa1..4ef39b6 100644 --- a/main.c +++ b/main.c @@ -245,6 +245,12 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev, if (unlikely(!pskb_may_pull(skb, 2))) goto err_free;
+ /* sometimes the mac_len value is not properly computed (e.g. when using + * batman-adv on top of a vlan interface), therefore we need to + * explicitly recompute it here + */ + skb_reset_mac_len(skb); + /* expect a valid ethernet header here. */ if (unlikely(skb->mac_len != ETH_HLEN || !skb_mac_header(skb))) goto err_free;
On Sat, Sep 15, 2012 at 02:34:21 +0200, Antonio Quartulli wrote:
It is possible that the mac_len is not properly exported because of strange device configuration (this behaviour has been observed when using batman-adv on top of a vlan interface). Therefore it is needed to explicitly recompute it at the very beginning of the rx path
Signed-off-by: Antonio Quartulli ordex@autistici.org
Instead of merging this patch I think that the best approach would be to fix the problem in VLAN code in the kernel. I'll try to send a patch to netdev.
Cheers,
Hey Antonio,
although I agree that it is the better way to get the fix in the kernel instead of batman-adv, it would be good to have a workaround in batman-adv for out-of-tree versions.
I've just sent a (pretty dirty) suggestion, another idea would be to get patches in the various distributions (openwrt, debian, gentoo, ...) - which would still leave the problem open for the next "source" release.
Cheers, Simon
On Sat, Sep 29, 2012 at 01:27:57PM +0200, Antonio Quartulli wrote:
On Sat, Sep 15, 2012 at 02:34:21 +0200, Antonio Quartulli wrote:
It is possible that the mac_len is not properly exported because of strange device configuration (this behaviour has been observed when using batman-adv on top of a vlan interface). Therefore it is needed to explicitly recompute it at the very beginning of the rx path
Signed-off-by: Antonio Quartulli ordex@autistici.org
Instead of merging this patch I think that the best approach would be to fix the problem in VLAN code in the kernel. I'll try to send a patch to netdev.
Cheers,
-- Antonio Quartulli
..each of us alone is worth nothing.. Ernesto "Che" Guevara
It is possible that the mac_len is not properly exported because of strange device configuration (this behaviour has been observed when using batman-adv on top of a vlan interface). Therefore it is needed to explicitly recompute it at the very beginning of the rx path.
This is done by appending the recompute function to the skb_share_mac() function, hence the "dirty hack" in the subject. We expect this problem to be fixed in linux 3.8 and above.
Reported-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de ---
this is a rewrite of Antonios patch "batman-adv: recompute mac_len at the beginning of the rx path". It is intended to fix the issue for out-of-kernel releases, without hurting the in-kernel code too much. Obviously, this patch won't go upstream into the kernel. --- compat.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/compat.h b/compat.h index 14969e0..dca9685 100644 --- a/compat.h +++ b/compat.h @@ -159,4 +159,19 @@ static inline void eth_hw_addr_random(struct net_device *dev)
#endif /* < KERNEL_VERSION(3, 5, 0) */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 8, 0) + +/* hack for not correctly set mac_len. This may happen for some special + * configurations like batman-adv on VLANs. + * + * This is pretty dirty, but we only use skb_share_check() in main.c right + * before mac_len is checked, and the recomputation shouldn't hurt too much. + */ +#define skb_share_check(skb, b) \ + skb_share_check(skb, b); \ + if (skb) \ + skb_reset_mac_len(skb) + +#endif /* < KERNEL_VERSION(3, 8, 0) */ + #endif /* _NET_BATMAN_ADV_COMPAT_H_ */
On Tuesday, October 02, 2012 04:53:28 Simon Wunderlich wrote:
+#define skb_share_check(skb, b) \
skb_share_check(skb, b); \if (skb) \skb_reset_mac_len(skb)+#endif /* < KERNEL_VERSION(3, 8, 0) */
Has this patch been tested ? Our skb_share_check() call is this: skb = skb_share_check(skb, GFP_ATOMIC);
Now we replace this function call with 2 function calls and 2 return values ?
Cheers, Marek
Hello Marek,
On Tue, Oct 02, 2012 at 01:16:40PM +0800, Marek Lindner wrote:
On Tuesday, October 02, 2012 04:53:28 Simon Wunderlich wrote:
+#define skb_share_check(skb, b) \
skb_share_check(skb, b); \if (skb) \skb_reset_mac_len(skb)+#endif /* < KERNEL_VERSION(3, 8, 0) */
Has this patch been tested ? Our skb_share_check() call is this: skb = skb_share_check(skb, GFP_ATOMIC);
Now we replace this function call with 2 function calls and 2 return values ?
I have not tested in a real machine, but only the first function will return the skb. The second part is a separate statement. I've checked it with gcc -E (preprocessor only), these lines in main.c will expand to:
hard_iface = ({ const typeof( ((struct batadv_hard_iface *)0)->batman_adv_ptype ) *__mptr = (ptype); (struct batadv_hard_iface *)( (char *)__mptr - __builtin_offsetof(struct batadv_hard_iface,batman_adv_ptype) );}) ; skb = skb_share_check(skb, ((( gfp_t)0x20u))); if (skb) skb_reset_mac_len(skb);
if (!skb) goto err_out;
As you can see, the skb = skb_share_check() statement stays at it is, and the reset_mac_len() is done afterwards.
Cheers, Simon
On Monday 01 October 2012 22:53:28 Simon Wunderlich wrote:
It is possible that the mac_len is not properly exported because of strange device configuration (this behaviour has been observed when using batman-adv on top of a vlan interface). Therefore it is needed to explicitly recompute it at the very beginning of the rx path.
This is done by appending the recompute function to the skb_share_mac() function, hence the "dirty hack" in the subject. We expect this problem to be fixed in linux 3.8 and above.
Reported-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de
this is a rewrite of Antonios patch "batman-adv: recompute mac_len at the beginning of the rx path". It is intended to fix the issue for out-of-kernel releases, without hurting the in-kernel code too much. Obviously, this patch won't go upstream into the kernel.
compat.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/compat.h b/compat.h index 14969e0..dca9685 100644 --- a/compat.h +++ b/compat.h @@ -159,4 +159,19 @@ static inline void eth_hw_addr_random(struct net_device *dev)
#endif /* < KERNEL_VERSION(3, 5, 0) */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 8, 0)
+/* hack for not correctly set mac_len. This may happen for some special
- configurations like batman-adv on VLANs.
- This is pretty dirty, but we only use skb_share_check() in main.c right
- before mac_len is checked, and the recomputation shouldn't hurt too
much. + */ +#define skb_share_check(skb, b) \
- skb_share_check(skb, b); \
- if (skb) \
skb_reset_mac_len(skb)+#endif /* < KERNEL_VERSION(3, 8, 0) */
#endif /* _NET_BATMAN_ADV_COMPAT_H_ */
Can we try a more sane solution like
#define skb_share_check(skb, b) \ ({ \ struct sk_buff *_t_skb; \ _t_skb = skb_share_check(skb, b); \ if (_t_skb) \ skb_reset_mac_len(_t_skb); \ _t_skb; \ })
Please test whether this thing really works and compiles. I just wrote it doing something else and never compiled it.
Kind regards, Sven
It is possible that the mac_len is not properly exported because of strange device configuration (this behaviour has been observed when using batman-adv on top of a vlan interface). Therefore it is needed to explicitly recompute it at the very beginning of the rx path.
This is done by appending the recompute function to the skb_share_mac() function, hence the "dirty hack" in the subject. We expect this problem to be fixed in linux 3.8 and above.
Reported-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de ---
this is a rewrite of Antonios patch "batman-adv: recompute mac_len at the beginning of the rx path". It is intended to fix the issue for out-of-kernel releases, without hurting the in-kernel code too much. Obviously, this patch won't go upstream into the kernel.
This version includes Svens "more sane" version, and also has implements skb_reset_mac_len() for kernels prior to 3.0. --- compat.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/compat.h b/compat.h index 14969e0..47223f5 100644 --- a/compat.h +++ b/compat.h @@ -137,6 +137,11 @@ void batadv_free_rcu_neigh_node(struct rcu_head *rcu); void batadv_free_rcu_tt_local_entry(struct rcu_head *rcu); void batadv_free_rcu_backbone_gw(struct rcu_head *rcu);
+static inline void skb_reset_mac_len(struct sk_buff *skb) +{ + skb->mac_len = skb->network_header - skb->mac_header; +} + #endif /* < KERNEL_VERSION(3, 0, 0) */
@@ -159,4 +164,23 @@ static inline void eth_hw_addr_random(struct net_device *dev)
#endif /* < KERNEL_VERSION(3, 5, 0) */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 8, 0) + +/* hack for not correctly set mac_len. This may happen for some special + * configurations like batman-adv on VLANs. + * + * This is pretty dirty, but we only use skb_share_check() in main.c right + * before mac_len is checked, and the recomputation shouldn't hurt too much. + */ +#define skb_share_check(skb, b) \ + ({ \ + struct sk_buff *_t_skb; \ + _t_skb = skb_share_check(skb, b); \ + if (_t_skb) \ + skb_reset_mac_len(_t_skb); \ + _t_skb; \ + }) + +#endif /* < KERNEL_VERSION(3, 8, 0) */ + #endif /* _NET_BATMAN_ADV_COMPAT_H_ */
On Sunday, October 14, 2012 20:46:57 Simon Wunderlich wrote:
It is possible that the mac_len is not properly exported because of strange device configuration (this behaviour has been observed when using batman-adv on top of a vlan interface). Therefore it is needed to explicitly recompute it at the very beginning of the rx path.
This is done by appending the recompute function to the skb_share_mac() function, hence the "dirty hack" in the subject. We expect this problem to be fixed in linux 3.8 and above.
Reported-by: Antonio Quartulli ordex@autistici.org Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de
this is a rewrite of Antonios patch "batman-adv: recompute mac_len at the beginning of the rx path". It is intended to fix the issue for out-of-kernel releases, without hurting the in-kernel code too much. Obviously, this patch won't go upstream into the kernel.
This version includes Svens "more sane" version, and also has implements skb_reset_mac_len() for kernels prior to 3.0.
compat.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
Applied in revision a6ad857.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Marek Lindner -
Simon Wunderlich -
Sven Eckelmann