Message ID | 20190730105417.14538-1-pablo@netfilter.org |
---|---|
State | Changes Requested |
Delegated to: | Pablo Neira |
Headers | show |
Series | [nf] netfilter: nf_tables: map basechain priority to hardware priority | expand |
On Tue, Jul 30, 2019 at 12:54:17PM +0200, Pablo Neira Ayuso wrote: [...] > @@ -180,6 +181,29 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain, > return 0; > } > > +/* Available priorities for hardware offload range: -8192..8191 */ > +#define NFT_BASECHAIN_OFFLOAD_PRIO_MAX (SHRT_MAX / 4) > +#define NFT_BASECHAIN_OFFLOAD_PRIO_MIN (SHRT_MIN / 4) > +#define NFT_BASECHAIN_OFFLOAD_PRIO_RANGE (USHRT_MAX / 4) > +/* tcf_auto_prio() uses 0xC000 as base, then subtract one for each new chain. */ > +#define NFT_BASECHAIN_OFFLOAD_HW_PRIO_BASE (0xC000 + 1) > + > +u16 nft_chain_offload_priority(struct nft_base_chain *basechain) > +{ > + u16 prio; > + > + if (basechain->ops.priority < NFT_BASECHAIN_OFFLOAD_PRIO_MIN || > + basechain->ops.priority > NFT_BASECHAIN_OFFLOAD_PRIO_MAX) > + return 0; > + > + /* map netfilter chain priority to hardware priority. */ > + prio = basechain->ops.priority + > + NFT_BASECHAIN_OFFLOAD_PRIO_MAX + > + NFT_BASECHAIN_OFFLOAD_HW_PRIO_BASE; > + > + return prio; This function should actually return: return prio << 16; > +} > + > static int nft_flow_offload_rule(struct nft_trans *trans, > enum flow_cls_command command) > {
On Tue, Jul 30, 2019 at 01:18:00PM +0200, Pablo Neira Ayuso wrote: > On Tue, Jul 30, 2019 at 12:54:17PM +0200, Pablo Neira Ayuso wrote: > [...] > > @@ -180,6 +181,29 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain, > > return 0; > > } > > > > +/* Available priorities for hardware offload range: -8192..8191 */ > > +#define NFT_BASECHAIN_OFFLOAD_PRIO_MAX (SHRT_MAX / 4) > > +#define NFT_BASECHAIN_OFFLOAD_PRIO_MIN (SHRT_MIN / 4) > > +#define NFT_BASECHAIN_OFFLOAD_PRIO_RANGE (USHRT_MAX / 4) > > +/* tcf_auto_prio() uses 0xC000 as base, then subtract one for each new chain. */ > > +#define NFT_BASECHAIN_OFFLOAD_HW_PRIO_BASE (0xC000 + 1) > > + > > +u16 nft_chain_offload_priority(struct nft_base_chain *basechain) > > +{ > > + u16 prio; > > + > > + if (basechain->ops.priority < NFT_BASECHAIN_OFFLOAD_PRIO_MIN || > > + basechain->ops.priority > NFT_BASECHAIN_OFFLOAD_PRIO_MAX) > > + return 0; > > + > > + /* map netfilter chain priority to hardware priority. */ > > + prio = basechain->ops.priority + > > + NFT_BASECHAIN_OFFLOAD_PRIO_MAX + > > + NFT_BASECHAIN_OFFLOAD_HW_PRIO_BASE; > > + > > + return prio; > > This function should actually return: > > return prio << 16; Better shift it from here: static void nft_flow_offload_common_init(struct flow_cls_common_offload *common, __be16 proto, int priority, struct netlink_ext_ack *extack) { common->protocol = proto; common->prio = priority << 16; common->extack = extack; } Drivers assume tc handle format.
On Tue, Jul 30, 2019 at 12:54:17PM +0200, Pablo Neira Ayuso wrote: > This patch maps basechain netfilter priorities from -8192 to 8191 to > hardware priority 0xC000 + 1. tcf_auto_prio() uses 0xC000 if the user > specifies no priority, then it subtract 1 for each new tcf_proto object. > This patch uses the hardware priority range from 0xC000 to 0xFFFF for > netfilter. This makes more sense, thanks Pablo. Nit below. > +u16 nft_chain_offload_priority(struct nft_base_chain *basechain) > +{ > + u16 prio; > + > + if (basechain->ops.priority < NFT_BASECHAIN_OFFLOAD_PRIO_MIN || > + basechain->ops.priority > NFT_BASECHAIN_OFFLOAD_PRIO_MAX) > + return 0; > + > + /* map netfilter chain priority to hardware priority. */ > + prio = basechain->ops.priority + > + NFT_BASECHAIN_OFFLOAD_PRIO_MAX + > + NFT_BASECHAIN_OFFLOAD_HW_PRIO_BASE; Weird indent here. > + > + return prio; > +}
On Tue, 30 Jul 2019 12:54:17 +0200, Pablo Neira Ayuso wrote: > This patch maps basechain netfilter priorities from -8192 to 8191 to > hardware priority 0xC000 + 1. tcf_auto_prio() uses 0xC000 if the user > specifies no priority, then it subtract 1 for each new tcf_proto object. > This patch uses the hardware priority range from 0xC000 to 0xFFFF for > netfilter. > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > This follows a rather conservative approach, I could just expose the > 2^16 hardware priority range, but we may need to split this priority > range among the ethtool_rx, tc and netfilter subsystems to start with > and it should be possible to extend the priority range later on. > > By netfilter priority, I'm refering to the basechain priority: > > add chain x y { type filter hook ingress device eth0 priority 0; } > ^^^^^^^^^^^ > > This is no transparently mapped to hardware, this patch shifts it to > make it fit into the 0xC000 + 1 .. 0xFFFF hardware priority range. Mmm.. so the ordering of tables is intended to be decided by priority and not block type (nft, tc, ethtool)? I was always expecting we would just follow the software order when it comes to inter-subsystem decisions. So ethtool first, then XDP, then TC, then nft, then bridging etc. TC vs NFT based on: static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc, struct packet_type **ppt_prev) { ... if (static_branch_unlikely(&ingress_needed_key)) { skb = sch_handle_ingress(skb, &pt_prev, &ret, orig_dev); if (!skb) goto out; if (nf_ingress(skb, &pt_prev, &ret, orig_dev) < 0) goto out; } Are they solid use cases for choosing the ordering arbitrarily?
diff --git a/include/net/netfilter/nf_tables_offload.h b/include/net/netfilter/nf_tables_offload.h index 3196663a10e3..2d497394021e 100644 --- a/include/net/netfilter/nf_tables_offload.h +++ b/include/net/netfilter/nf_tables_offload.h @@ -73,4 +73,6 @@ int nft_flow_rule_offload_commit(struct net *net); (__reg)->key = __key; \ memset(&(__reg)->mask, 0xff, (__reg)->len); +u16 nft_chain_offload_priority(struct nft_base_chain *basechain); + #endif diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 93647fdf435c..9ee6db9a668d 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1669,6 +1669,10 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask, chain->flags |= NFT_BASE_CHAIN | flags; basechain->policy = NF_ACCEPT; + if (chain->flags & NFT_CHAIN_HW_OFFLOAD && + !nft_chain_offload_priority(basechain)) + return -EOPNOTSUPP; + flow_block_init(&basechain->flow_block); } else { chain = kzalloc(sizeof(*chain), GFP_KERNEL); diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c index ec70978eba5a..df8427ba857c 100644 --- a/net/netfilter/nf_tables_offload.c +++ b/net/netfilter/nf_tables_offload.c @@ -156,10 +156,11 @@ void nft_offload_update_dependency(struct nft_offload_ctx *ctx, } static void nft_flow_offload_common_init(struct flow_cls_common_offload *common, - __be16 proto, - struct netlink_ext_ack *extack) + __be16 proto, int priority, + struct netlink_ext_ack *extack) { common->protocol = proto; + common->prio = priority; common->extack = extack; } @@ -180,6 +181,29 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain, return 0; } +/* Available priorities for hardware offload range: -8192..8191 */ +#define NFT_BASECHAIN_OFFLOAD_PRIO_MAX (SHRT_MAX / 4) +#define NFT_BASECHAIN_OFFLOAD_PRIO_MIN (SHRT_MIN / 4) +#define NFT_BASECHAIN_OFFLOAD_PRIO_RANGE (USHRT_MAX / 4) +/* tcf_auto_prio() uses 0xC000 as base, then subtract one for each new chain. */ +#define NFT_BASECHAIN_OFFLOAD_HW_PRIO_BASE (0xC000 + 1) + +u16 nft_chain_offload_priority(struct nft_base_chain *basechain) +{ + u16 prio; + + if (basechain->ops.priority < NFT_BASECHAIN_OFFLOAD_PRIO_MIN || + basechain->ops.priority > NFT_BASECHAIN_OFFLOAD_PRIO_MAX) + return 0; + + /* map netfilter chain priority to hardware priority. */ + prio = basechain->ops.priority + + NFT_BASECHAIN_OFFLOAD_PRIO_MAX + + NFT_BASECHAIN_OFFLOAD_HW_PRIO_BASE; + + return prio; +} + static int nft_flow_offload_rule(struct nft_trans *trans, enum flow_cls_command command) { @@ -200,7 +224,9 @@ static int nft_flow_offload_rule(struct nft_trans *trans, if (flow) proto = flow->proto; - nft_flow_offload_common_init(&cls_flow.common, proto, &extack); + nft_flow_offload_common_init(&cls_flow.common, proto, + nft_chain_offload_priority(basechain), + &extack); cls_flow.command = command; cls_flow.cookie = (unsigned long) rule; if (flow)
This patch maps basechain netfilter priorities from -8192 to 8191 to hardware priority 0xC000 + 1. tcf_auto_prio() uses 0xC000 if the user specifies no priority, then it subtract 1 for each new tcf_proto object. This patch uses the hardware priority range from 0xC000 to 0xFFFF for netfilter. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- This follows a rather conservative approach, I could just expose the 2^16 hardware priority range, but we may need to split this priority range among the ethtool_rx, tc and netfilter subsystems to start with and it should be possible to extend the priority range later on. By netfilter priority, I'm refering to the basechain priority: add chain x y { type filter hook ingress device eth0 priority 0; } ^^^^^^^^^^^ This is no transparently mapped to hardware, this patch shifts it to make it fit into the 0xC000 + 1 .. 0xFFFF hardware priority range. include/net/netfilter/nf_tables_offload.h | 2 ++ net/netfilter/nf_tables_api.c | 4 ++++ net/netfilter/nf_tables_offload.c | 32 ++++++++++++++++++++++++++++--- 3 files changed, 35 insertions(+), 3 deletions(-)