diff mbox series

[net,1/4] netfilter: nftables_offload: set address type in control dissector

Message ID 20201121123601.21733-2-pablo@netfilter.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net,1/4] netfilter: nftables_offload: set address type in control dissector | expand

Checks

Context Check Description
netdev/cover_letter warning Series does not have a cover letter
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 19 this patch: 19
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: 'succesfully' may be misspelled - perhaps 'successfully'? WARNING: line length of 81 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 19 this patch: 19
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Pablo Neira Ayuso Nov. 21, 2020, 12:35 p.m. UTC
If the address type is missing through the control dissector, then
matching on IPv4 and IPv6 addresses does not work. Set it accordingly so
rules that specify an IP address succesfully match on packets.

Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables_offload.h |  4 ++++
 net/netfilter/nf_tables_offload.c         | 18 ++++++++++++++++++
 net/netfilter/nft_payload.c               |  4 ++++
 3 files changed, 26 insertions(+)

Comments

Jakub Kicinski Nov. 22, 2020, 12:44 a.m. UTC | #1
On Sat, 21 Nov 2020 13:35:58 +0100 Pablo Neira Ayuso wrote:
> If the address type is missing through the control dissector, then
> matching on IPv4 and IPv6 addresses does not work.

Doesn't work where? Are you talking about a specific driver?

> Set it accordingly so
> rules that specify an IP address succesfully match on packets.
> 
> Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/net/netfilter/nf_tables_offload.h |  4 ++++
>  net/netfilter/nf_tables_offload.c         | 18 ++++++++++++++++++
>  net/netfilter/nft_payload.c               |  4 ++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/include/net/netfilter/nf_tables_offload.h b/include/net/netfilter/nf_tables_offload.h
> index ea7d1d78b92d..bddd34c5bd79 100644
> --- a/include/net/netfilter/nf_tables_offload.h
> +++ b/include/net/netfilter/nf_tables_offload.h
> @@ -37,6 +37,7 @@ void nft_offload_update_dependency(struct nft_offload_ctx *ctx,
>  
>  struct nft_flow_key {
>  	struct flow_dissector_key_basic			basic;
> +	struct flow_dissector_key_control		control;
>  	union {
>  		struct flow_dissector_key_ipv4_addrs	ipv4;
>  		struct flow_dissector_key_ipv6_addrs	ipv6;
> @@ -62,6 +63,9 @@ struct nft_flow_rule {
>  
>  #define NFT_OFFLOAD_F_ACTION	(1 << 0)
>  
> +void nft_flow_rule_set_addr_type(struct nft_flow_rule *flow,
> +				 enum flow_dissector_key_id addr_type);
> +
>  struct nft_rule;
>  struct nft_flow_rule *nft_flow_rule_create(struct net *net, const struct nft_rule *rule);
>  void nft_flow_rule_destroy(struct nft_flow_rule *flow);
> diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
> index 9f625724a20f..9a3c5ac057b6 100644
> --- a/net/netfilter/nf_tables_offload.c
> +++ b/net/netfilter/nf_tables_offload.c
> @@ -28,6 +28,24 @@ static struct nft_flow_rule *nft_flow_rule_alloc(int num_actions)
>  	return flow;
>  }
>  
> +void nft_flow_rule_set_addr_type(struct nft_flow_rule *flow,
> +				 enum flow_dissector_key_id addr_type)
> +{
> +	struct nft_flow_match *match = &flow->match;
> +	struct nft_flow_key *mask = &match->mask;
> +	struct nft_flow_key *key = &match->key;
> +
> +	if (match->dissector.used_keys & BIT(FLOW_DISSECTOR_KEY_CONTROL))
> +		return;
> +
> +	key->control.addr_type = addr_type;
> +	mask->control.addr_type = 0xffff;
> +	match->dissector.used_keys |= BIT(FLOW_DISSECTOR_KEY_CONTROL);
> +	match->dissector.offset[FLOW_DISSECTOR_KEY_CONTROL] =
> +		offsetof(struct nft_flow_key, control);

Why is this injecting the match conditionally?

> +}
> +EXPORT_SYMBOL_GPL(nft_flow_rule_set_addr_type);

And why is this exported? 

nf_tables-objs := nf_tables_core.o nf_tables_api.o nft_chain_filter.o \
		  nf_tables_trace.o nft_immediate.o nft_cmp.o nft_range.o \
		  nft_bitwise.o nft_byteorder.o nft_payload.o nft_lookup.o \
                                                ^^^^^^^^^^^^^
		  nft_dynset.o nft_meta.o nft_rt.o nft_exthdr.o \
		  nft_chain_route.o nf_tables_offload.o \
                                    ^^^^^^^^^^^^^^^^^^^
		  nft_set_hash.o nft_set_bitmap.o nft_set_rbtree.o \
		  nft_set_pipapo.o

These are linked together.
Pablo Neira Ayuso Nov. 22, 2020, 10:49 a.m. UTC | #2
On Sat, Nov 21, 2020 at 04:44:42PM -0800, Jakub Kicinski wrote:
> On Sat, 21 Nov 2020 13:35:58 +0100 Pablo Neira Ayuso wrote:
> > If the address type is missing through the control dissector, then
> > matching on IPv4 and IPv6 addresses does not work.
> 
> Doesn't work where? Are you talking about a specific driver?

No.

It does not work for any kind of match, the control flow dissector
needs to be set on.
diff mbox series

Patch

diff --git a/include/net/netfilter/nf_tables_offload.h b/include/net/netfilter/nf_tables_offload.h
index ea7d1d78b92d..bddd34c5bd79 100644
--- a/include/net/netfilter/nf_tables_offload.h
+++ b/include/net/netfilter/nf_tables_offload.h
@@ -37,6 +37,7 @@  void nft_offload_update_dependency(struct nft_offload_ctx *ctx,
 
 struct nft_flow_key {
 	struct flow_dissector_key_basic			basic;
+	struct flow_dissector_key_control		control;
 	union {
 		struct flow_dissector_key_ipv4_addrs	ipv4;
 		struct flow_dissector_key_ipv6_addrs	ipv6;
@@ -62,6 +63,9 @@  struct nft_flow_rule {
 
 #define NFT_OFFLOAD_F_ACTION	(1 << 0)
 
+void nft_flow_rule_set_addr_type(struct nft_flow_rule *flow,
+				 enum flow_dissector_key_id addr_type);
+
 struct nft_rule;
 struct nft_flow_rule *nft_flow_rule_create(struct net *net, const struct nft_rule *rule);
 void nft_flow_rule_destroy(struct nft_flow_rule *flow);
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 9f625724a20f..9a3c5ac057b6 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -28,6 +28,24 @@  static struct nft_flow_rule *nft_flow_rule_alloc(int num_actions)
 	return flow;
 }
 
+void nft_flow_rule_set_addr_type(struct nft_flow_rule *flow,
+				 enum flow_dissector_key_id addr_type)
+{
+	struct nft_flow_match *match = &flow->match;
+	struct nft_flow_key *mask = &match->mask;
+	struct nft_flow_key *key = &match->key;
+
+	if (match->dissector.used_keys & BIT(FLOW_DISSECTOR_KEY_CONTROL))
+		return;
+
+	key->control.addr_type = addr_type;
+	mask->control.addr_type = 0xffff;
+	match->dissector.used_keys |= BIT(FLOW_DISSECTOR_KEY_CONTROL);
+	match->dissector.offset[FLOW_DISSECTOR_KEY_CONTROL] =
+		offsetof(struct nft_flow_key, control);
+}
+EXPORT_SYMBOL_GPL(nft_flow_rule_set_addr_type);
+
 struct nft_flow_rule *nft_flow_rule_create(struct net *net,
 					   const struct nft_rule *rule)
 {
diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index dcd3c7b8a367..bbf811d030d5 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -244,6 +244,7 @@  static int nft_payload_offload_ip(struct nft_offload_ctx *ctx,
 
 		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4, src,
 				  sizeof(struct in_addr), reg);
+		nft_flow_rule_set_addr_type(flow, FLOW_DISSECTOR_KEY_IPV4_ADDRS);
 		break;
 	case offsetof(struct iphdr, daddr):
 		if (priv->len != sizeof(struct in_addr))
@@ -251,6 +252,7 @@  static int nft_payload_offload_ip(struct nft_offload_ctx *ctx,
 
 		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4, dst,
 				  sizeof(struct in_addr), reg);
+		nft_flow_rule_set_addr_type(flow, FLOW_DISSECTOR_KEY_IPV4_ADDRS);
 		break;
 	case offsetof(struct iphdr, protocol):
 		if (priv->len != sizeof(__u8))
@@ -280,6 +282,7 @@  static int nft_payload_offload_ip6(struct nft_offload_ctx *ctx,
 
 		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6, src,
 				  sizeof(struct in6_addr), reg);
+		nft_flow_rule_set_addr_type(flow, FLOW_DISSECTOR_KEY_IPV6_ADDRS);
 		break;
 	case offsetof(struct ipv6hdr, daddr):
 		if (priv->len != sizeof(struct in6_addr))
@@ -287,6 +290,7 @@  static int nft_payload_offload_ip6(struct nft_offload_ctx *ctx,
 
 		NFT_OFFLOAD_MATCH(FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6, dst,
 				  sizeof(struct in6_addr), reg);
+		nft_flow_rule_set_addr_type(flow, FLOW_DISSECTOR_KEY_IPV6_ADDRS);
 		break;
 	case offsetof(struct ipv6hdr, nexthdr):
 		if (priv->len != sizeof(__u8))