From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH RFC,net-next 04/10] cls_flower: add translator to flow_action representation Date: Wed, 26 Sep 2018 08:47:55 -0700 Message-ID: <20180926084755.301f4aa6@cakuba.netronome.com> References: <20180925192001.2482-1-pablo@netfilter.org> <20180925192001.2482-5-pablo@netfilter.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, davem@davemloft.net, thomas.lendacky@amd.com, f.fainelli@gmail.com, ariel.elior@cavium.com, michael.chan@broadcom.com, santosh@chelsio.com, madalin.bucur@nxp.com, yisen.zhuang@huawei.com, salil.mehta@huawei.com, jeffrey.t.kirsher@intel.com, tariqt@mellanox.com, saeedm@mellanox.com, jiri@mellanox.com, idosch@mellanox.com, ganeshgr@chelsio.com, linux-net-drivers@solarflare.com, peppe.cavallaro@st.com, alexandre.torgue@st.com, joabreu@synopsys.com, grygorii.strashko@ti.com, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com To: Pablo Neira Ayuso Return-path: Received: from mail-qk1-f194.google.com ([209.85.222.194]:35231 "EHLO mail-qk1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727136AbeIZWBw (ORCPT ); Wed, 26 Sep 2018 18:01:52 -0400 Received: by mail-qk1-f194.google.com with SMTP id a189-v6so6483107qkb.2 for ; Wed, 26 Sep 2018 08:48:01 -0700 (PDT) In-Reply-To: <20180925192001.2482-5-pablo@netfilter.org> Sender: netdev-owner@vger.kernel.org List-ID: On Tue, 25 Sep 2018 21:19:55 +0200, Pablo Neira Ayuso wrote: > This implements TC action to flow_action translation from cls_flower. > > Signed-off-by: Pablo Neira Ayuso > --- > net/sched/cls_flower.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 123 insertions(+), 1 deletion(-) > > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c > index e1dd60a2ecb8..a96a80f01c6d 100644 > --- a/net/sched/cls_flower.c > +++ b/net/sched/cls_flower.c > @@ -28,6 +28,14 @@ > > #include > #include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > > struct fl_flow_key { > int indev_ifindex; > @@ -101,6 +109,7 @@ struct cls_fl_filter { > u32 in_hw_count; > struct rcu_work rwork; > struct net_device *hw_dev; > + struct flow_action action; > }; > > static const struct rhashtable_params mask_ht_params = { > @@ -294,6 +303,107 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f, > tcf_block_offload_dec(block, &f->flags); > } > > +static int fl_hw_setup_action(struct flow_action *flow_action, > + const struct tcf_exts *exts) The function doesn't seem very flower-specific? > +{ > + const struct tc_action *act; > + int num_acts = 0, i, j, k; > + > + if (!exts) > + return 0; > + > + tcf_exts_for_each_action(i, act, exts) { > + if (is_tcf_pedit(act)) > + num_acts += tcf_pedit_nkeys(act); > + else > + num_acts++; > + } > + > + if (!num_acts) > + return 0; > + > + if (flow_action_init(flow_action, num_acts) < 0) > + return -ENOMEM; > + > + j = 0; > + tcf_exts_for_each_action(i, act, exts) { > + struct flow_action_key *key; > + > + key = &flow_action->keys[j]; > + if (is_tcf_gact_ok(act)) { > + key->id = FLOW_ACTION_KEY_ACCEPT; > + } else if (is_tcf_gact_shot(act)) { > + key->id = FLOW_ACTION_KEY_DROP; > + } else if (is_tcf_gact_trap(act)) { > + key->id = FLOW_ACTION_KEY_TRAP; > + } else if (is_tcf_gact_goto_chain(act)) { > + key->id = FLOW_ACTION_KEY_GOTO; > + key->chain_index = tcf_gact_goto_chain_index(act); > + } else if (is_tcf_mirred_egress_redirect(act)) { > + key->id = FLOW_ACTION_KEY_REDIRECT; > + key->dev = tcf_mirred_dev(act); > + } else if (is_tcf_mirred_egress_mirror(act)) { > + key->id = FLOW_ACTION_KEY_MIRRED; > + key->dev = tcf_mirred_dev(act); > + } else if (is_tcf_vlan(act)) { > + switch (tcf_vlan_action(act)) { > + case TCA_VLAN_ACT_PUSH: > + key->id = FLOW_ACTION_KEY_VLAN_PUSH; > + key->vlan.vid = tcf_vlan_push_vid(act); > + key->vlan.proto = tcf_vlan_push_proto(act); > + key->vlan.prio = tcf_vlan_push_prio(act); > + break; > + case TCA_VLAN_ACT_POP: > + key->id = FLOW_ACTION_KEY_VLAN_POP; > + break; > + case TCA_VLAN_ACT_MODIFY: > + key->id = FLOW_ACTION_KEY_VLAN_MANGLE; > + key->vlan.vid = tcf_vlan_push_vid(act); > + key->vlan.proto = tcf_vlan_push_proto(act); > + key->vlan.prio = tcf_vlan_push_prio(act); > + break; > + } > + } else if (is_tcf_tunnel_set(act)) { > + key->id = FLOW_ACTION_KEY_TUNNEL_ENCAP; > + key->tunnel = tcf_tunnel_info(act); > + } else if (is_tcf_tunnel_release(act)) { > + key->id = FLOW_ACTION_KEY_TUNNEL_DECAP; > + key->tunnel = tcf_tunnel_info(act); > + } else if (is_tcf_pedit(act)) { > + for (k = 0; k < tcf_pedit_nkeys(act); k++) { > + switch (tcf_pedit_cmd(act, k)) { > + case TCA_PEDIT_KEY_EX_CMD_SET: > + key->id = FLOW_ACTION_KEY_MANGLE; > + break; > + case TCA_PEDIT_KEY_EX_CMD_ADD: > + key->id = FLOW_ACTION_KEY_ADD; > + break; > + default: > + WARN_ON_ONCE(1); > + break; > + } > + > + key->mangle.htype = tcf_pedit_htype(act, k); > + key->mangle.mask = tcf_pedit_mask(act, k); > + key->mangle.val = tcf_pedit_val(act, k); > + key->mangle.offset = tcf_pedit_offset(act, k); > + key = &flow_action->keys[++j]; > + } > + } else if (is_tcf_csum(act)) { > + key->id = FLOW_ACTION_KEY_CSUM; > + key->csum_flags = tcf_csum_update_flags(act); > + } else if (is_tcf_skbedit_mark(act)) { > + key->id = FLOW_ACTION_KEY_MARK; > + key->mark = tcf_skbedit_mark(act); > + } Why the permissiveness? Shouldn't we error out if we there is an unknown action? else -EOPNOTSUPP + extack? > + > + if (!is_tcf_pedit(act)) > + j++; > + } > + > + return 0; > +} > + > static int fl_hw_replace_filter(struct tcf_proto *tp, > struct cls_fl_filter *f, > struct netlink_ext_ack *extack)