| Message ID | 1334825108.2395.28.camel@edumazet-glaptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 19/04/2012 10:45, Eric Dumazet wrote: >> Maybe you should try latest kernel, because we fixed some bugs lately. > > Oh well, at first glance nf_bridge_unshare() is buggy, not sure if this > can help your bug, but its another step. > I'm recompiling 3.3.2 with this new patch. I'll let you know. Thanks you, Massimo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19/04/2012 10:45, Eric Dumazet wrote: > Oh well, at first glance nf_bridge_unshare() is buggy, not sure if this > can help your bug, but its another step. > > > [PATCH] bridge: fix nf_bridge_unshare() > > If memory allocation failed, return an error. > > If not, skb->nf_bridge should be updated to point to the copy, not old > info, or bad things can happen. > > Signed-off-by: Eric Dumazet<eric.dumazet@gmail.com> > --- > net/bridge/br_netfilter.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c > index dec4f38..b7c2cec 100644 > --- a/net/bridge/br_netfilter.c > +++ b/net/bridge/br_netfilter.c > @@ -185,21 +185,20 @@ static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb) > return skb->nf_bridge; > } > > -static inline struct nf_bridge_info *nf_bridge_unshare(struct sk_buff *skb) > + > +static inline int nf_bridge_unshare(struct sk_buff *skb) > { > - struct nf_bridge_info *nf_bridge = skb->nf_bridge; > + struct nf_bridge_info *copy, *nf_bridge = skb->nf_bridge; > > if (atomic_read(&nf_bridge->use)> 1) { > - struct nf_bridge_info *tmp = nf_bridge_alloc(skb); > - > - if (tmp) { > - memcpy(tmp, nf_bridge, sizeof(struct nf_bridge_info)); > - atomic_set(&tmp->use, 1); > - } > + copy = kmemdup(nf_bridge, sizeof(*nf_bridge), GFP_ATOMIC); > + if (!copy) > + return -ENOMEM; > + atomic_set(©->use, 1); > nf_bridge_put(nf_bridge); > - nf_bridge = tmp; > + skb->nf_bridge = copy; > } > - return nf_bridge; > + return 0; > } > > static inline void nf_bridge_push_encap_header(struct sk_buff *skb) > @@ -744,8 +743,9 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb, > return NF_ACCEPT; > > /* Need exclusive nf_bridge_info since we might have multiple > - * different physoutdevs. */ > - if (!nf_bridge_unshare(skb)) > + * different physoutdevs. > + */ > + if (nf_bridge_unshare(skb)) > return NF_DROP; > > parent = bridge_parent(out); > > Hello, Eric, i applied this patch and Peters last patch to a 3.3.2 kernel. The result was a bit disappointing because the step was backwards. Locally, from the same machine, i could ping each IP of each tun interface used by any virtual server. From the LAN such addresses were not pingable while the ip address of the bridge was reachable. Max -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2012-04-20 at 12:02 +0200, Massimo Cetra wrote: > > i applied this patch and Peters last patch to a 3.3.2 kernel. > The result was a bit disappointing because the step was backwards. > > Locally, from the same machine, i could ping each IP of each tun > interface used by any virtual server. > > From the LAN such addresses were not pingable while the ip address of > the bridge was reachable. > > Max I dont know, this code is crap and should be fixed. This nf_bridge_unshare() is obviously buggy as hell. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index dec4f38..b7c2cec 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -185,21 +185,20 @@ static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb) return skb->nf_bridge; } -static inline struct nf_bridge_info *nf_bridge_unshare(struct sk_buff *skb) + +static inline int nf_bridge_unshare(struct sk_buff *skb) { - struct nf_bridge_info *nf_bridge = skb->nf_bridge; + struct nf_bridge_info *copy, *nf_bridge = skb->nf_bridge; if (atomic_read(&nf_bridge->use) > 1) { - struct nf_bridge_info *tmp = nf_bridge_alloc(skb); - - if (tmp) { - memcpy(tmp, nf_bridge, sizeof(struct nf_bridge_info)); - atomic_set(&tmp->use, 1); - } + copy = kmemdup(nf_bridge, sizeof(*nf_bridge), GFP_ATOMIC); + if (!copy) + return -ENOMEM; + atomic_set(©->use, 1); nf_bridge_put(nf_bridge); - nf_bridge = tmp; + skb->nf_bridge = copy; } - return nf_bridge; + return 0; } static inline void nf_bridge_push_encap_header(struct sk_buff *skb) @@ -744,8 +743,9 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb, return NF_ACCEPT; /* Need exclusive nf_bridge_info since we might have multiple - * different physoutdevs. */ - if (!nf_bridge_unshare(skb)) + * different physoutdevs. + */ + if (nf_bridge_unshare(skb)) return NF_DROP; parent = bridge_parent(out);