Re: error: 'struct net_device' has no member named 'nf_hooks_ingress'

[Sergey Senozhatsky]

On (10/06/16 06:11), Eric Dumazet wrote:
> On Wed, 2016-10-05 at 22:56 +0200, Michal Sojka wrote:
> 
> > this commit is now in mainline as
> > e3b37f11e6e4e6b6f02cc762f182ce233d2c1c9d and it breaks my build:
> > 
> > net/netfilter/core.c: In function 'nf_set_hooks_head':
> >     net/netfilter/core.c:96:3: error: 'struct net_device' has no member 
> > named 'nf_hooks_ingress'
> > 
> > Are the fixes (see below) on the way to mainline too?
> 
> Yes the fixes are already in nf tree and _will_ get pushed.
> 
> Pablo and David are attending netdev 1.2 in Tokyo and have obligations.
> 
> https://git.kernel.org/cgit/linux/kernel/git/pablo/nf-next.git/

well, I did my best to avoid it, but the guys didn't even bother to
reply. pushing a knowingly broken patch that
a) kills the build
b) introduces a race
c) requires two "Fixes:" followup patches
to the main line despite the fact that those problems were discovered
at linux-next stage is totally un-cool.

-ss

Re: error: 'struct net_device' has no member named 'nf_hooks_ingress'

[Eric Dumazet]

On Wed, 2016-10-05 at 22:56 +0200, Michal Sojka wrote:

> this commit is now in mainline as
> e3b37f11e6e4e6b6f02cc762f182ce233d2c1c9d and it breaks my build:
> 
> net/netfilter/core.c: In function 'nf_set_hooks_head':
> net/netfilter/core.c:96:3: error: 'struct net_device' has no member named 
> 'nf_hooks_ingress'
> 
> Are the fixes (see below) on the way to mainline too?

Yes the fixes are already in nf tree and _will_ get pushed.

Pablo and David are attending netdev 1.2 in Tokyo and have obligations.

https://git.kernel.org/cgit/linux/kernel/git/pablo/nf-next.git/

Thanks.

error: 'struct net_device' has no member named 'nf_hooks_ingress'

[Michal Sojka]

Hi,

On Tue, Oct 04 2016, Sergey Senozhatsky wrote:
> On (09/27/16 19:03), Sergey Senozhatsky wrote:
>> Hello,
>> 
>> On (09/27/16 16:40), Stephen Rothwell wrote:
>> > 
>> > Changes since 20160923:
>> > 
>> 
>> seems that commit e3b37f11e6e4e6b6 ("netfilter: replace list_head with
>> single linked list") breaks the build on !CONFIG_NETFILTER_INGRESS systems
>> accessing ->nf_hooks_ingress

this commit is now in mainline as
e3b37f11e6e4e6b6f02cc762f182ce233d2c1c9d and it breaks my build:

net/netfilter/core.c: In function 'nf_set_hooks_head':
net/netfilter/core.c:96:3: error: 'struct net_device' has no member named 
'nf_hooks_ingress'

Are the fixes (see below) on the way to mainline too?

Thanks.
-Michal



>> 
>> static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
>>  struct nf_hook_entry *entry)
>> {
>>switch (reg->pf) {
>>case NFPROTO_NETDEV:
>>/* We already checked in nf_register_net_hook() that this is
>> * used from ingress.
>> */
>>rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
>>  
>
>
> so I see two commits in linux-next now that fix the commit in question in
> two patches
>
>  : commit 7816ec564ec40ae20bb7925f733a181cad0cc491 ("netfilter: accommodate
>  : different kconfig in nf_set_hooks_head")
>  :
>  :When CONFIG_NETFILTER_INGRESS is unset (or no), we need to handle
>  :the request for registration properly by dropping the hook.  This
>  :releases the entry during the set.
>  :
>  :Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked 
> list")
>
> and
>
>  : commit 5119e4381a90fabd3442bde02707cbd9e5d7367a ("netfilter: Fix potential
>  : null pointer dereference")
>  :
>  :It's possible for nf_hook_entry_head to return NULL.  If two
>  :nf_unregister_net_hook calls happen simultaneously with a single hook
>  :entry in the list, both will enter the nf_hook_mutex critical section.
>  :The first will successfully delete the head, but the second will see
>  :this NULL pointer and attempt to dereference.
>  :
>  :This fix ensures that no null pointer dereference could occur when such
>  :a condition happens.
>  :
>  :Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked 
> list")
>
>
> do you guys plan to fold those into "e3b37f11e6e4" (a preferred way)
> or will send it out as 3 separate patches (um, why) ?
>
>   -ss