netdev - Re: [PATCH net-next,v3 11/11] netfilter: nf_tables: add hardware offload support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190708184437.4d29648a@cakuba.netronome.com>
Date:   Mon, 8 Jul 2019 18:44:37 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        thomas.lendacky@....com, f.fainelli@...il.com,
        ariel.elior@...ium.com, michael.chan@...adcom.com,
        madalin.bucur@....com, yisen.zhuang@...wei.com,
        salil.mehta@...wei.com, jeffrey.t.kirsher@...el.com,
        tariqt@...lanox.com, saeedm@...lanox.com, jiri@...lanox.com,
        idosch@...lanox.com, peppe.cavallaro@...com,
        grygorii.strashko@...com, andrew@...n.ch, vivien.didelot@...il.com,
        alexandre.torgue@...com, joabreu@...opsys.com,
        linux-net-drivers@...arflare.com, ogerlitz@...lanox.com,
        Manish.Chopra@...ium.com, marcelo.leitner@...il.com,
        mkubecek@...e.cz, venkatkumar.duvvuru@...adcom.com,
        maxime.chevallier@...tlin.com, cphealy@...il.com,
        netfilter-devel@...r.kernel.org
Subject: Re: [PATCH net-next,v3 11/11] netfilter: nf_tables: add hardware
 offload support

On Mon,  8 Jul 2019 18:06:13 +0200, Pablo Neira Ayuso wrote:
> This patch adds hardware offload support for nftables through the
> existing netdev_ops->ndo_setup_tc() interface, the TC_SETUP_CLSFLOWER
> classifier and the flow rule API. This hardware offload support is
> available for the NFPROTO_NETDEV family and the ingress hook.
> 
> Each nftables expression has a new ->offload interface, that is used to
> populate the flow rule object that is attached to the transaction
> object.
> 
> There is a new per-table NFT_TABLE_F_HW flag, that is set on to offload
> an entire table, including all of its chains.
> 
> This patch supports for basic metadata (layer 3 and 4 protocol numbers),
> 5-tuple payload matching and the accept/drop actions; this also includes
> basechain hardware offload only.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@...filter.org>

Any particular reason to not fence this off with a device feature
(ethtool -k)?  Then you wouldn't need that per-driver list abomination
until drivers start advertising it..  IDK if we want the per-device
offload enable flags or not in general, it seems like a good idea in
general for admin to be able to disable offload per device 🤷

> +static int nft_flow_offload_rule(struct nft_trans *trans,
> +				 enum tc_fl_command command)
> +{
> +	struct nft_flow_rule *flow = nft_trans_flow_rule(trans);
> +	struct nft_rule *rule = nft_trans_rule(trans);
> +	struct tc_cls_flower_offload cls_flower = {};
> +	struct nft_base_chain *basechain;
> +	struct netlink_ext_ack extack;
> +	__be16 proto = ETH_P_ALL;
> +
> +	if (!nft_is_base_chain(trans->ctx.chain))
> +		return -EOPNOTSUPP;
> +
> +	basechain = nft_base_chain(trans->ctx.chain);
> +
> +	if (flow)
> +		proto = flow->proto;
> +
> +	nft_flow_offload_common_init(&cls_flower.common, proto, &extack);
> +	cls_flower.command = command;
> +	cls_flower.cookie = (unsigned long) rule;
> +	if (flow)
> +		cls_flower.rule = flow->rule;
> +
> +	return nft_setup_cb_call(basechain, TC_SETUP_CLSFLOWER, &cls_flower);
> +}

Are we 100% okay with using TC cls_flower structures and defines in nft
code?