Re: [netfilter-core] [nf-next:master 5/7] ./usr/include/linux/netfilter/nf_osf.h:73: userspace cannot reference function or variable defined in the kernel

2018-07-30 Thread Florian Westphal
kbuild test robot  wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git 
> master
> head:   4ed8eb6570a49931c705512060acd50058d61616
> commit: f9324952088f1cd62ea4addf9ff532f1e6452a22 [5/7] netfilter: 
> nfnetlink_osf: extract nfnetlink_subsystem code from xt_osf.c
> config: i386-randconfig-a1-07310851 (attached as .config)
> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
> reproduce:
> git checkout f9324952088f1cd62ea4addf9ff532f1e6452a22
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
> >> ./usr/include/linux/netfilter/nf_osf.h:73: userspace cannot reference 
> >> function or variable defined in the kernel

Fernando, this is because of

+extern struct list_head nf_osf_fingers[2];

You either need to find another (non UAPI) header to place this in, or,
alternatively, consider wrapping it in
#ifdef __KERNEL__

Can you make a patch?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[nf-next:master 5/7] ./usr/include/linux/netfilter/nf_osf.h:73: userspace cannot reference function or variable defined in the kernel

2018-07-30 Thread kbuild test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
head:   4ed8eb6570a49931c705512060acd50058d61616
commit: f9324952088f1cd62ea4addf9ff532f1e6452a22 [5/7] netfilter: 
nfnetlink_osf: extract nfnetlink_subsystem code from xt_osf.c
config: i386-randconfig-a1-07310851 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout f9324952088f1cd62ea4addf9ff532f1e6452a22
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

>> ./usr/include/linux/netfilter/nf_osf.h:73: userspace cannot reference 
>> function or variable defined in the kernel

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip


Re: [Bug 200651] New: cgroups iptables-restor: vmalloc: allocation failure

2018-07-30 Thread Georgi Nikolov
On 07/30/2018 09:38 PM, Michal Hocko wrote:
> On Mon 30-07-18 18:54:24, Georgi Nikolov wrote:
> [...]
>> No i was wrong. The regression starts actually with 0537250fdc6c8.
>> - old code, which opencodes kvmalloc, is masking error but error is there
>> - kvmalloc without GFP_NORETRY works fine, but probably can consume a
>> lot of memory - commit: eacd86ca3b036
>> - kvmalloc with GFP_NORETRY shows error - commit: 0537250fdc6c8
> OK.
>
 What is correct way to fix it.
 - inside xt_alloc_table_info remove GFP_NORETRY from kvmalloc or add
 this flag only for sizes bigger than some threshold
>>> This would reintroduce issue fixed by 0537250fdc6c8. Note that
>>> kvmalloc(GFP_KERNEL | __GFP_NORETRY) is more or less equivalent to the
>>> original code (well, except for __GFP_NOWARN).
>> So probably we should pass GFP_NORETRY only for large requests (above
>> some threshold).
> What would be the treshold? This is not really my area so I just wanted
> to keep the original code semantic.
>  
 - inside kvmalloc_node remove GFP_NORETRY from
 __vmalloc_node_flags_caller (i don't know if it honors this flag, or
 the problem is elsewhere)
>>> No, not really. This is basically equivalent to kvmalloc(GFP_KERNEL).
>>>
>>> I strongly suspect that this is not a regression in this code but rather
>>> a side effect of larger memory fragmentation caused by something else.
>>> In any case do you see this failure also without artificial test case
>>> with a standard workload?
>> Yes i can see failures with standard workload, in fact it was hard to
>> reproduce it.
>> Here is the error from production servers where allocation is smaller:
>> iptables: vmalloc: allocation failure, allocated 131072 of 225280 bytes,
>> mode:0x14010c0(GFP_KERNEL|__GFP_NORETRY), nodemask=(null)
>>
>> I didn't understand if vmalloc honors GFP_NORETRY.
> 0537250fdc6c8 changelog tries to explain. kvmalloc doesn't really
> support the GFP_NORETRY remantic because that would imply the request
> wouldn't trigger the oom killer but in rare cases this might happen
> (e.g. when page tables are allocated because those are hardcoded GFP_KERNEL).
>
> That being said, I have no objection to use GFP_KERNEL if it helps real
> workloads but we probably need some cap...

Probably Vlastimil Babka can propose some limit:

On Thu 26-07-18 09:18:57, Vlastimil Babka wrote:
This is likely the kvmalloc() in xt_alloc_table_info(). Between 4.13 and
4.17 it shouldn't use __GFP_NORETRY, but looks like commit 0537250fdc6c
("netfilter: x_tables: make allocation less aggressive") was backported
to 4.14. Removing __GFP_NORETRY might help here, but bring back other
issues. Less than 4MB is not that much though, maybe find some "sane"
limit and use __GFP_NORETRY only above that?


Regards,

--
Georgi Nikolov


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bug 200651] New: cgroups iptables-restor: vmalloc: allocation failure

2018-07-30 Thread Michal Hocko
On Mon 30-07-18 18:54:24, Georgi Nikolov wrote:
[...]
> No i was wrong. The regression starts actually with 0537250fdc6c8.
> - old code, which opencodes kvmalloc, is masking error but error is there
> - kvmalloc without GFP_NORETRY works fine, but probably can consume a
> lot of memory - commit: eacd86ca3b036
> - kvmalloc with GFP_NORETRY shows error - commit: 0537250fdc6c8

OK.

> >> What is correct way to fix it.
> >> - inside xt_alloc_table_info remove GFP_NORETRY from kvmalloc or add
> >> this flag only for sizes bigger than some threshold
> > This would reintroduce issue fixed by 0537250fdc6c8. Note that
> > kvmalloc(GFP_KERNEL | __GFP_NORETRY) is more or less equivalent to the
> > original code (well, except for __GFP_NOWARN).
> 
> So probably we should pass GFP_NORETRY only for large requests (above
> some threshold).

What would be the treshold? This is not really my area so I just wanted
to keep the original code semantic.
 
> >> - inside kvmalloc_node remove GFP_NORETRY from
> >> __vmalloc_node_flags_caller (i don't know if it honors this flag, or
> >> the problem is elsewhere)
> > No, not really. This is basically equivalent to kvmalloc(GFP_KERNEL).
> >
> > I strongly suspect that this is not a regression in this code but rather
> > a side effect of larger memory fragmentation caused by something else.
> > In any case do you see this failure also without artificial test case
> > with a standard workload?
> 
> Yes i can see failures with standard workload, in fact it was hard to
> reproduce it.
> Here is the error from production servers where allocation is smaller:
> iptables: vmalloc: allocation failure, allocated 131072 of 225280 bytes,
> mode:0x14010c0(GFP_KERNEL|__GFP_NORETRY), nodemask=(null)
> 
> I didn't understand if vmalloc honors GFP_NORETRY.

0537250fdc6c8 changelog tries to explain. kvmalloc doesn't really
support the GFP_NORETRY remantic because that would imply the request
wouldn't trigger the oom killer but in rare cases this might happen
(e.g. when page tables are allocated because those are hardcoded GFP_KERNEL).

That being said, I have no objection to use GFP_KERNEL if it helps real
workloads but we probably need some cap...
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bug 200651] New: cgroups iptables-restor: vmalloc: allocation failure

2018-07-30 Thread Georgi Nikolov
On 07/30/2018 04:57 PM, Michal Hocko wrote:

> On Mon 30-07-18 16:37:07, Georgi Nikolov wrote:
>> On 07/26/2018 12:02 PM, Georgi Nikolov wrote:
> [...]
>>> Here is the patch applied to this version which masks errors:
>>>
>>> --- net/netfilter/x_tables.c    2018-06-18 14:18:21.138347416 +0300
>>> +++ net/netfilter/x_tables.c    2018-07-26 11:58:01.721932962 +0300
>>> @@ -1059,9 +1059,19 @@
>>>   * than shoot all processes down before realizing there is nothing
>>>   * more to reclaim.
>>>   */
>>> -    info = kvmalloc(sz, GFP_KERNEL | __GFP_NORETRY);
>>> +/*    info = kvmalloc(sz, GFP_KERNEL | __GFP_NORETRY);
>>>  if (!info)
>>>      return NULL;
>>> +*/
>>> +
>>> +    if (sz <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
>>> +        info = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
>>> +    if (!info) {
>>> +        info = __vmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY,
>>> +         PAGE_KERNEL);
>>> +        if (!info)
>>> +        return NULL;
>>> +    }
>>>  
>>>  memset(info, 0, sizeof(*info));
>>>  info->size = size;
>>>
>>>
>>> I will try to reproduce it with only
>>>
>>> info = kvmalloc(sz, GFP_KERNEL);
>>>
>>> Regards,
>>>
>>> --
>>> Georgi Nikolov
>>>
>> Hello,
>>
>> Without GFP_NORETRY problem disappears.
> Hmm, there are two allocation paths which have __GFP_NORETRY here.
> I expect you have removed both of them, right?
>
> kvmalloc implicitly performs __GFP_NORETRY on kmalloc path but it
> doesn't have it for the vmalloc fallback. This would match
> kvmalloc(GFP_KERNEL). I thought you were testing this code path
> previously but there is some confusion flying around because you have
> claimed that the regressions started with eacd86ca3b036. If the
> regression is really with __GFP_NORETRY being used for the vmalloc
> fallback which would be kvmalloc(GFP_KERNEL | __GFP_NORETRY) then
> I am still confused because that would match the original code.

No i was wrong. The regression starts actually with 0537250fdc6c8.
- old code, which opencodes kvmalloc, is masking error but error is there
- kvmalloc without GFP_NORETRY works fine, but probably can consume a
lot of memory - commit: eacd86ca3b036
- kvmalloc with GFP_NORETRY shows error - commit: 0537250fdc6c8

>> What is correct way to fix it.
>> - inside xt_alloc_table_info remove GFP_NORETRY from kvmalloc or add
>> this flag only for sizes bigger than some threshold
> This would reintroduce issue fixed by 0537250fdc6c8. Note that
> kvmalloc(GFP_KERNEL | __GFP_NORETRY) is more or less equivalent to the
> original code (well, except for __GFP_NOWARN).

So probably we should pass GFP_NORETRY only for large requests (above
some threshold).

>
>> - inside kvmalloc_node remove GFP_NORETRY from
>> __vmalloc_node_flags_caller (i don't know if it honors this flag, or
>> the problem is elsewhere)
> No, not really. This is basically equivalent to kvmalloc(GFP_KERNEL).
>
> I strongly suspect that this is not a regression in this code but rather
> a side effect of larger memory fragmentation caused by something else.
> In any case do you see this failure also without artificial test case
> with a standard workload?

Yes i can see failures with standard workload, in fact it was hard to
reproduce it.
Here is the error from production servers where allocation is smaller:
iptables: vmalloc: allocation failure, allocated 131072 of 225280 bytes,
mode:0x14010c0(GFP_KERNEL|__GFP_NORETRY), nodemask=(null)

I didn't understand if vmalloc honors GFP_NORETRY.

Regards,

--
Georgi Nikolov

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3 nf-next v2] netfilter: nf_osf: rename nf_osf.c to nfnetlink_osf.c

2018-07-30 Thread Jan Engelhardt


On Monday 2018-07-30 14:23, Pablo Neira Ayuso wrote:
>
>Right, but we cannot assume users use iptables, they may develop their
>own applications based on our binary interface.

But if iptables does the file copy, and nftables does the same copy,
then by that pattern, all applications, his own including, should do
such a copy. Which is how I remember it...
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bug 200651] New: cgroups iptables-restor: vmalloc: allocation failure

2018-07-30 Thread Michal Hocko
On Mon 30-07-18 16:37:07, Georgi Nikolov wrote:
> On 07/26/2018 12:02 PM, Georgi Nikolov wrote:
[...]
> > Here is the patch applied to this version which masks errors:
> >
> > --- net/netfilter/x_tables.c    2018-06-18 14:18:21.138347416 +0300
> > +++ net/netfilter/x_tables.c    2018-07-26 11:58:01.721932962 +0300
> > @@ -1059,9 +1059,19 @@
> >   * than shoot all processes down before realizing there is nothing
> >   * more to reclaim.
> >   */
> > -    info = kvmalloc(sz, GFP_KERNEL | __GFP_NORETRY);
> > +/*    info = kvmalloc(sz, GFP_KERNEL | __GFP_NORETRY);
> >  if (!info)
> >      return NULL;
> > +*/
> > +
> > +    if (sz <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
> > +        info = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
> > +    if (!info) {
> > +        info = __vmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY,
> > +         PAGE_KERNEL);
> > +        if (!info)
> > +        return NULL;
> > +    }
> >  
> >  memset(info, 0, sizeof(*info));
> >  info->size = size;
> >
> >
> > I will try to reproduce it with only
> >
> > info = kvmalloc(sz, GFP_KERNEL);
> >
> > Regards,
> >
> > --
> > Georgi Nikolov
> >
> 
> Hello,
> 
> Without GFP_NORETRY problem disappears.

Hmm, there are two allocation paths which have __GFP_NORETRY here.
I expect you have removed both of them, right?

kvmalloc implicitly performs __GFP_NORETRY on kmalloc path but it
doesn't have it for the vmalloc fallback. This would match
kvmalloc(GFP_KERNEL). I thought you were testing this code path
previously but there is some confusion flying around because you have
claimed that the regressions started with eacd86ca3b036. If the
regression is really with __GFP_NORETRY being used for the vmalloc
fallback which would be kvmalloc(GFP_KERNEL | __GFP_NORETRY) then
I am still confused because that would match the original code.

> What is correct way to fix it.
> - inside xt_alloc_table_info remove GFP_NORETRY from kvmalloc or add
> this flag only for sizes bigger than some threshold

This would reintroduce issue fixed by 0537250fdc6c8. Note that
kvmalloc(GFP_KERNEL | __GFP_NORETRY) is more or less equivalent to the
original code (well, except for __GFP_NOWARN).

> - inside kvmalloc_node remove GFP_NORETRY from
> __vmalloc_node_flags_caller (i don't know if it honors this flag, or
> the problem is elsewhere)

No, not really. This is basically equivalent to kvmalloc(GFP_KERNEL).

I strongly suspect that this is not a regression in this code but rather
a side effect of larger memory fragmentation caused by something else.
In any case do you see this failure also without artificial test case
with a standard workload?
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3 nf-next v2] netfilter: nf_osf: rename nf_osf.c to nfnetlink_osf.c

2018-07-30 Thread Pablo Neira Ayuso
On Mon, Jul 23, 2018 at 12:26:31PM +0200, Jan Engelhardt wrote:
> On Monday 2018-07-23 12:06, Pablo Neira Ayuso wrote:
> 
> >On Fri, Jul 20, 2018 at 04:41:11PM +0200, Fernando Fernandez Mancera wrote:
> >> Rename nf_osf.c to nfnetlink_osf.c as we introduce nfnetlink_osf which is
> >> the OSF infraestructure.
> >> 
> >> Signed-off-by: Fernando Fernandez Mancera 
> >> ---
> >>  .../linux/netfilter/{nf_osf.h => nfnetlink_osf.h} |  2 +-
> >>  .../linux/netfilter/{nf_osf.h => nfnetlink_osf.h} |  6 +++---
> >>  include/uapi/linux/netfilter/xt_osf.h |  2 +-
> >>  net/netfilter/Kconfig | 15 ++-
> >>  net/netfilter/Makefile|  2 +-
> >>  net/netfilter/{nf_osf.c => nfnetlink_osf.c}   |  2 +-
> >>  6 files changed, 17 insertions(+), 12 deletions(-)
> >>  rename include/linux/netfilter/{nf_osf.h => nfnetlink_osf.h} (95%)
> >>  rename include/uapi/linux/netfilter/{nf_osf.h => nfnetlink_osf.h} (96%)
> >
> >The uapi file we cannot rename. Anything in the uapi folder is set in
> >stone forever.
> 
> Userspace such as iptables keeps copies of header files so that it 
> always builds no matter what set of files the kernel offers.

Right, but we cannot assume users use iptables, they may develop their
own applications based on our binary interface. But I think this
rename is fine given uapi/linux/netfilter/nf_osf.h did not have users
so far, given that this is only useful for nft_osf.

Fernando, would you send a patch rename the header to nfnetlink_osf.h
as Jan suggested? I suggest you add a Suggested-by: tag. Explain in
the commit message that first client of this is nft_osf.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH libnftnl] Expose socket mark via socket expression

2018-07-30 Thread Pablo Neira Ayuso
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 libnftnl] Add tproxy support

2018-07-30 Thread Pablo Neira Ayuso
Applied, thanks Mate.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH libnftnl v3] expr: add osf support

2018-07-30 Thread Pablo Neira Ayuso
Applied, thanks Fernando.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 nf-next] netfilter: Add native tproxy support for nf_tables

2018-07-30 Thread Pablo Neira Ayuso
On Sat, Jul 28, 2018 at 04:37:25PM +0200, Máté Eckl wrote:
> A great portion of the code is taken from xt_TPROXY.c
> 
> There are some changes compared to the iptables implementation:
>  - tproxy statement is not terminal here
>  - Either address or port has to be specified, but at least one of them
>is necessary. If one of them is not specified, the evaluation will be
>performed with the original attribute of the packet (ie. target port
>is not specified => the packet's dport will be used).
> 
> To make this work in inet tables, the tproxy structure has a family
> member (typically called priv->family) which is not necessarily equal to
> ctx->family.
> 
> priv->family can have three values legally:
>  - NFPROTO_IPV4 if the table family is ip OR if table family is inet,
>but an ipv4 address is specified as a target address. The rule only
>evaluates ipv4 packets in this case.
>  - NFPROTO_IPV6 if the table family is ip6 OR if table family is inet,
>but an ipv6 address is specified as a target address. The rule only
>evaluates ipv6 packets in this case.
>  - NFPROTO_UNSPEC if the table family is inet AND if only the port is
>specified. The rule will evaluate both ipv4 and ipv6 packets.

Applied, thanks Mate.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3 nf-next v3] netfilter: nft_osf: implement Passive OS fingerprint module in nft_osf

2018-07-30 Thread Pablo Neira Ayuso
On Wed, Jul 25, 2018 at 01:32:46AM +0200, Fernando Fernandez Mancera wrote:
> Add basic module functions into nft_osf.[ch] in order to implement OSF
> module in nf_tables.

Applied, thanks Fernando.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3 nf-next v3] netfilter: nfnetlink_osf: extract nfnetlink_subsystem code from xt_osf.c

2018-07-30 Thread Pablo Neira Ayuso
On Wed, Jul 25, 2018 at 01:32:45AM +0200, Fernando Fernandez Mancera wrote:
> Move nfnetlink osf subsystem from xt_osf.c to standalone module so we can
> reuse it from the new nft_ost extension.

Also applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3 nf-next v3] netfilter: nf_osf: rename nf_osf.c to nfnetlink_osf.c

2018-07-30 Thread Pablo Neira Ayuso
On Wed, Jul 25, 2018 at 01:32:44AM +0200, Fernando Fernandez Mancera wrote:
> Rename nf_osf.c to nfnetlink_osf.c as we introduce nfnetlink_osf which is
> the OSF infraestructure.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html