Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Jul 03, 2019 at 03:08:01PM +0300, Nikolay Aleksandrov wrote:
> On 28/06/2019 03:49, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.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@ucloud.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.