netdev - Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 3 Jul 2019 14:23:13 +0200
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Cc:     wenxu@...oud.cn, fw@...len.de, netfilter-devel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add
 NFT_META_BRI_IIFVPROTO support

On Wed, Jul 03, 2019 at 03:08:01PM +0300, Nikolay Aleksandrov wrote:
> On 28/06/2019 03:49, wenxu@...oud.cn wrote:
> > From: wenxu <wenxu@...oud.cn>
> > 
> > This patch provide a meta to get the bridge vlan proto
> > 
> > nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
> > 
> > Signed-off-by: wenxu <wenxu@...oud.cn>
> > ---
> >  include/uapi/linux/netfilter/nf_tables.h | 2 ++
> >  net/netfilter/nft_meta.c                 | 9 +++++++++
> >  2 files changed, 11 insertions(+)
> > 
> 
> Hi,
> When using the internal bridge API outside of the bridge I'd advise you to CC bridge
> maintainers as well.

Will keep this mind, thanks.

> This patch is clearly wrong since you cannot access the vlan
> fields directly because bridge vlan support might be disabled from the kernel config
> as Pablo has noticed as well. In general I'd try to avoid using the internal API directly,
> but that is a different matter.

BROPT_VLAN_ENABLED is exposed through netlink and sysfs, and this only
consults the value. I guess you refer to the fact that...

> Please consult with include/linux/if_bridge.h for exported
> functions that are supposed to be visible outside of the bridge, if you need anything else
> make sure to add support for it there. The usage of br_opt_get directly for example must
> be changed to br_vlan_enabled().

Indeed... this patch should be using br_vlan_enabled() instead.

Thanks.