| Message ID | 1293150436-12092-1-git-send-email-fw@strlen.de |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Dec 24, 2010 at 8:27 AM, Florian Westphal <fw@strlen.de> wrote: > broute table init hook sets up the "br_should_route_hook" pointer, > which then gets called from br_input. > > commit a386f99025f13b32502fe5dedf223c20d7283826 > (bridge: add proper RCU annotation to should_route_hook) > introduced a typedef, and then changed this to: > > br_should_route_hook_t *rhook; > [..] > rhook = rcu_dereference(br_should_route_hook); > if (*rhook(skb)) > > problem is that "br_should_route_hook" contains the address of the function, > so calling *rhook() results in kernel panic. > > Can't simply use "if (rhook(skb)", because br_should_route_hook_t *rhook > is not a function. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > > include/linux/if_bridge.h | 2 +- > net/bridge/br_input.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h > index f7e73c3..dd3f201 100644 > --- a/include/linux/if_bridge.h > +++ b/include/linux/if_bridge.h > @@ -103,7 +103,7 @@ struct __fdb_entry { > > extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *)); > > -typedef int (*br_should_route_hook_t)(struct sk_buff *skb); > +typedef int br_should_route_hook_t(struct sk_buff *skb); > extern br_should_route_hook_t __rcu *br_should_route_hook; > > #endif > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index 6f6d8e1..1d677bf 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -178,7 +178,7 @@ forward: > case BR_STATE_FORWARDING: > rhook = rcu_dereference(br_should_route_hook); > if (rhook) { > - if ((*rhook)(skb)) > + if (rhook(skb)) I don't think this change is necessary. Would you like to remove this change and test again? > return skb; > dest = eth_hdr(skb)->h_dest; > }
Changli Gao <xiaosuo@gmail.com> wrote: > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > index 6f6d8e1..1d677bf 100644 > > --- a/net/bridge/br_input.c > > +++ b/net/bridge/br_input.c > > @@ -178,7 +178,7 @@ forward: > > case BR_STATE_FORWARDING: > > rhook = rcu_dereference(br_should_route_hook); > > if (rhook) { > > - if ((*rhook)(skb)) > > + if (rhook(skb)) > > I don't think this change is necessary. Would you like to remove this > change and test again? Its not needed, but i consider it less confusing. I do not have a strong preference, though. -- 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/include/linux/if_bridge.h b/include/linux/if_bridge.h index f7e73c3..dd3f201 100644 --- a/include/linux/if_bridge.h +++ b/include/linux/if_bridge.h @@ -103,7 +103,7 @@ struct __fdb_entry { extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *)); -typedef int (*br_should_route_hook_t)(struct sk_buff *skb); +typedef int br_should_route_hook_t(struct sk_buff *skb); extern br_should_route_hook_t __rcu *br_should_route_hook; #endif diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 6f6d8e1..1d677bf 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -178,7 +178,7 @@ forward: case BR_STATE_FORWARDING: rhook = rcu_dereference(br_should_route_hook); if (rhook) { - if ((*rhook)(skb)) + if (rhook(skb)) return skb; dest = eth_hdr(skb)->h_dest; }
broute table init hook sets up the "br_should_route_hook" pointer, which then gets called from br_input. commit a386f99025f13b32502fe5dedf223c20d7283826 (bridge: add proper RCU annotation to should_route_hook) introduced a typedef, and then changed this to: br_should_route_hook_t *rhook; [..] rhook = rcu_dereference(br_should_route_hook); if (*rhook(skb)) problem is that "br_should_route_hook" contains the address of the function, so calling *rhook() results in kernel panic. Can't simply use "if (rhook(skb)", because br_should_route_hook_t *rhook is not a function. Signed-off-by: Florian Westphal <fw@strlen.de> --- include/linux/if_bridge.h | 2 +- net/bridge/br_input.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)