[PATCH] netfilter: nfnetlink_log:add support for VLAN information

[PATCH] netfilter: nfnetlink_log:add support for VLAN information

[Michael Braun]

Currently, there is no vlan information (e.g. when used with a vlan aware
bridge) passed to userspache, HWHEADER will contain an 08 00 (ip) suffix
even for tagged ip packets.

Therefore, add an extra netlink attribute that passes the vlan tag to
userspace. Userspace might need to handle PCP/DEI included in this field.

Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
 include/uapi/linux/netfilter/nfnetlink_log.h | 1 +
 net/netfilter/nf_log_common.c                | 2 ++
 net/netfilter/nfnetlink_log.c                | 6 ++++++
 3 files changed, 9 insertions(+)

diff --git a/include/uapi/linux/netfilter/nfnetlink_log.h b/include/uapi/linux/netfilter/nfnetlink_log.h
index 20983cb195a0..d15f74d47f48 100644
--- a/include/uapi/linux/netfilter/nfnetlink_log.h
+++ b/include/uapi/linux/netfilter/nfnetlink_log.h
@@ -54,6 +54,7 @@ enum nfulnl_attr_type {
 	NFULA_HWLEN,			/* hardware header length */
 	NFULA_CT,                       /* nf_conntrack_netlink.h */
 	NFULA_CT_INFO,                  /* enum ip_conntrack_info */
+	NFULA_VLAN_TAG,                 /* __u16 vlan tag */
 
 	__NFULA_MAX
 };
diff --git a/net/netfilter/nf_log_common.c b/net/netfilter/nf_log_common.c
index ae5628ddbe6d..57c4cc8fbead 100644
--- a/net/netfilter/nf_log_common.c
+++ b/net/netfilter/nf_log_common.c
@@ -160,6 +160,8 @@ nf_log_dump_packet_common(struct nf_log_buf *m, u_int8_t pf,
 	       '0' + loginfo->u.log.level, prefix,
 	       in ? in->name : "",
 	       out ? out->name : "");
+	if (skb_vlan_tag_present(skb))
+		nf_log_buf_add(m, "VLAN=%d ", skb_vlan_tag_get_id(skb));
 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
 	physindev = nf_bridge_get_physindev(skb);
 	if (physindev && in != physindev)
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 6dee4f9a944c..f6fe0d760816 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -16,6 +16,7 @@
 #include <linux/skbuff.h>
 #include <linux/if_arp.h>
 #include <linux/init.h>
+#include <linux/if_vlan.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <linux/netdevice.h>
@@ -580,6 +581,11 @@ __build_packet_message(struct nfnl_log_net *log,
 				 NFULA_CT, NFULA_CT_INFO) < 0)
 		goto nla_put_failure;
 
+	if (skb_vlan_tag_present(skb) &&
+	    nla_put_be16(inst->skb, NFULA_VLAN_TAG,
+			 htons(skb_vlan_tag_get(skb))))
+		goto nla_put_failure;
+
 	if (data_len) {
 		struct nlattr *nla;
 		int size = nla_attr_size(data_len);
-- 
2.20.1

Re: [PATCH] netfilter: nfnetlink_log:add support for VLAN information

[Pablo Neira Ayuso]

On Mon, Aug 05, 2019 at 09:28:14AM +0200, Michael Braun wrote:
> Currently, there is no vlan information (e.g. when used with a vlan aware
> bridge) passed to userspache, HWHEADER will contain an 08 00 (ip) suffix
> even for tagged ip packets.
> 
> Therefore, add an extra netlink attribute that passes the vlan tag to
> userspace. Userspace might need to handle PCP/DEI included in this field.
> 
> Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
> ---
>  include/uapi/linux/netfilter/nfnetlink_log.h | 1 +
>  net/netfilter/nf_log_common.c                | 2 ++
>  net/netfilter/nfnetlink_log.c                | 6 ++++++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/include/uapi/linux/netfilter/nfnetlink_log.h b/include/uapi/linux/netfilter/nfnetlink_log.h
> index 20983cb195a0..d15f74d47f48 100644
> --- a/include/uapi/linux/netfilter/nfnetlink_log.h
> +++ b/include/uapi/linux/netfilter/nfnetlink_log.h
> @@ -54,6 +54,7 @@ enum nfulnl_attr_type {
>  	NFULA_HWLEN,			/* hardware header length */
>  	NFULA_CT,                       /* nf_conntrack_netlink.h */
>  	NFULA_CT_INFO,                  /* enum ip_conntrack_info */
> +	NFULA_VLAN_TAG,                 /* __u16 vlan tag */

Nitpick: Would you rename this to NFULA_VLAN_TCI? Both fields in the
vlan header refer to tag, either for protocol or control.

Thanks.

Re: [PATCH] netfilter: nfnetlink_log:add support for VLAN information

[Pablo Neira Ayuso]

On Mon, Aug 05, 2019 at 11:26:49AM +0200, Pablo Neira Ayuso wrote:
> On Mon, Aug 05, 2019 at 09:28:14AM +0200, Michael Braun wrote:
> > Currently, there is no vlan information (e.g. when used with a vlan aware
> > bridge) passed to userspache, HWHEADER will contain an 08 00 (ip) suffix
> > even for tagged ip packets.
> > 
> > Therefore, add an extra netlink attribute that passes the vlan tag to
> > userspace. Userspace might need to handle PCP/DEI included in this field.

Would you also send patches to update userspace? Thanks.

Re: [PATCH] netfilter: nfnetlink_log:add support for VLAN information

[Florian Westphal]

Michael Braun <michael-dev@fami-braun.de> wrote:
> Currently, there is no vlan information (e.g. when used with a vlan aware
> bridge) passed to userspache, HWHEADER will contain an 08 00 (ip) suffix
> even for tagged ip packets.
> 
> Therefore, add an extra netlink attribute that passes the vlan tag to
> userspace. Userspace might need to handle PCP/DEI included in this field.
> 
> Signed-off-by: Michael Braun <michael-dev@fami-braun.de>

nfqueue has nfqnl_put_bridge() helper which will plcae both tci and
proto in a nested attribute, I wonder if we can just re-use that?

(Yes, we need new attributes unfortunately).

Re: [PATCH] netfilter: nfnetlink_log:add support for VLAN information

[Pablo Neira Ayuso]

On Mon, Aug 05, 2019 at 12:35:16PM +0200, Florian Westphal wrote:
> Michael Braun <michael-dev@fami-braun.de> wrote:
> > Currently, there is no vlan information (e.g. when used with a vlan aware
> > bridge) passed to userspache, HWHEADER will contain an 08 00 (ip) suffix
> > even for tagged ip packets.
> > 
> > Therefore, add an extra netlink attribute that passes the vlan tag to
> > userspace. Userspace might need to handle PCP/DEI included in this field.
> > 
> > Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
> 
> nfqueue has nfqnl_put_bridge() helper which will plcae both tci and
> proto in a nested attribute, I wonder if we can just re-use that?
> 
> (Yes, we need new attributes unfortunately).

Indeed, something similar to nfqnl_put_bridge() would be great to get
nfnetlink_log in feature parity with nfnetlink_queue.

Thanks.