Message ID | 20211012081937.440411-8-cmi@nvidia.com |
---|---|
State | Superseded |
Headers | show |
Series | Add offload support for sFlow | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
See some small comments below... On 12 Oct 2021, at 10:19, Chris Mi wrote: > Create a unique group ID to map the sFlow info when offloading sFlow > action to TC. When showing the offloaded datapath flows, translate the > group ID from TC sample action to sFlow info using the mapping. > > Signed-off-by: Chris Mi <cmi@nvidia.com> > Reviewed-by: Eli Britstein <elibr@nvidia.com> > --- > NEWS | 1 + > lib/dpif-netlink.c | 1 + > lib/dpif-offload-netlink.c | 6 + > lib/dpif-offload-provider.h | 1 + > lib/netdev-offload-tc.c | 228 +++++++++++++++++++++++++++++++++--- > lib/netdev-offload.h | 3 + > lib/tc.c | 61 +++++++++- > lib/tc.h | 15 ++- > 8 files changed, 294 insertions(+), 22 deletions(-) > > diff --git a/NEWS b/NEWS > index 90f4b1590..d34c87b3c 100644 > --- a/NEWS > +++ b/NEWS > @@ -10,6 +10,7 @@ Post-v2.16.0 > limiting behavior. > * Add hardware offload support for matching IPv4/IPv6 frag types > (experimental). > + - Add sFlow offload support for kernel (netlink) datapath. > > > v2.16.0 - 16 Aug 2021 > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index 19ae543e6..3f51f319f 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -2234,6 +2234,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) > info.tunnel_csum_on = csum_on; > info.recirc_id_shared_with_tc = (dpif->user_features > & OVS_DP_F_TC_RECIRC_SHARING); > + info.psample_exists = dpif_offload_netlink_psample_exists(); I would just call dpif_offload_netlink_psample_supported() directly in the netdev_tc_flow_put() API. > err = netdev_flow_put(dev, &match, > CONST_CAST(struct nlattr *, put->actions), > put->actions_len, > diff --git a/lib/dpif-offload-netlink.c b/lib/dpif-offload-netlink.c > index 6ebf1aa0c..4db71ecea 100644 > --- a/lib/dpif-offload-netlink.c > +++ b/lib/dpif-offload-netlink.c > @@ -207,3 +207,9 @@ const struct dpif_offload_api dpif_offload_netlink = { > .sflow_recv_wait = dpif_offload_netlink_sflow_recv_wait, > .sflow_recv = dpif_offload_netlink_sflow_recv, > }; > + > +bool > +dpif_offload_netlink_psample_exists(void) > +{ > + return psample_sock != NULL; > +} I would call this dpif_offload_netlink_psample_supported() > diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h > index 0a0b71618..573f3add5 100644 > --- a/lib/dpif-offload-provider.h > +++ b/lib/dpif-offload-provider.h > @@ -69,6 +69,7 @@ int dpif_offload_sflow_recv(const struct dpif *dpif, > struct dpif_offload_sflow *sflow); > > #ifdef __linux__ > +bool dpif_offload_netlink_psample_exists(void); > extern const struct dpif_offload_api dpif_offload_netlink; > #endif > > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index 204d1c833..a47a8e2c1 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -20,6 +20,7 @@ > #include <linux/if_ether.h> > > #include "dpif.h" > +#include "dpif-offload-provider.h" > #include "hash.h" > #include "openvswitch/hmap.h" > #include "openvswitch/match.h" > @@ -1052,6 +1053,19 @@ parse_tc_flower_to_match(struct tc_flower *flower, > action = flower->actions; > for (i = 0; i < flower->action_count; i++, action++) { > switch (action->type) { > + case TC_ACT_SAMPLE: { > + const struct sgid_node *node; > + > + node = sgid_find(action->sample.group_id); > + if (!node) { > + VLOG_WARN("Can't find sample group ID data for ID: %u", > + action->sample.group_id); > + return ENOENT; > + } > + nl_msg_put(buf, node->sflow.action, > + node->sflow.action->nla_len); > + } > + break; > case TC_ACT_VLAN_POP: { > nl_msg_put_flag(buf, OVS_ACTION_ATTR_POP_VLAN); > } > @@ -1790,6 +1804,151 @@ parse_match_ct_state_to_flower(struct tc_flower *flower, struct match *match) > } > } > > +static int > +parse_userspace_attributes(const struct nlattr *actions, > + struct dpif_sflow_attr *sflow_attr) > +{ > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > + const struct nlattr *nla; > + unsigned int left; > + > + NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) { > + if (nl_attr_type(nla) == OVS_USERSPACE_ATTR_USERDATA) { > + struct user_action_cookie *cookie; > + > + cookie = CONST_CAST(struct user_action_cookie *, nl_attr_get(nla)); > + if (cookie->type == USER_ACTION_COOKIE_SFLOW) { > + sflow_attr->userdata = nla; > + return 0; > + } > + } > + } > + > + VLOG_DBG_RL(&rl, "Can't offload userspace action other than sFlow."); Log messages do not seem to end with a dot, so please remove it. > + return EOPNOTSUPP; > +} > + > +static int > +parse_sample_actions_attribute(const struct nlattr *actions, > + struct dpif_sflow_attr *sflow_attr) > +{ > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > + const struct nlattr *nla; > + unsigned int left; > + int err = EINVAL; > + > + NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) { > + if (nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) { > + err = parse_userspace_attributes(nla, sflow_attr); > + } else { > + /* We can't offload other nested actions */ > + VLOG_DBG_RL(&rl, "Can only offload OVS_ACTION_ATTR_USERSPACE" > + " attribute."); Log messages do not seem to end with a dot, so please remove it. > + return EINVAL; > + } > + } > + > + if (err) { > + VLOG_ERR_RL(&error_rl, "No OVS_ACTION_ATTR_USERSPACE attribute."); Log messages do not seem to end with a dot, so please remove it. > + } > + return err; > +} > + > +static int > +parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action, > + const struct nlattr *sample_action, > + const struct flow_tnl *tnl, uint32_t *group_id, > + const ovs_u128 *ufid) > +{ > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > + struct dpif_sflow_attr sflow_attr; > + const struct nlattr *nla; > + unsigned int left; > + int ret = EINVAL; > + > + if (*group_id) { > + VLOG_ERR_RL(&error_rl, " Only a single TC_SAMPLE action per flow is" > + " supported"); Please change it to below, so it’s easier to grep for: VLOG_ERR_RL(&error_rl, " Only a single TC_SAMPLE action per flow is supported"); > + return EOPNOTSUPP; > + } > + > + memset(&sflow_attr, 0, sizeof sflow_attr); > + sflow_attr.ufid = *ufid; > + sflow_attr.action = sample_action; > + > + if (flower->tunnel) { > + sflow_attr.tunnel = CONST_CAST(struct flow_tnl *, tnl); > + } > + > + NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) { > + if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) { > + ret = parse_sample_actions_attribute(nla, &sflow_attr); > + } else if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_PROBABILITY) { > + tc_action->type = TC_ACT_SAMPLE; > + tc_action->sample.rate = UINT32_MAX / nl_attr_get_u32(nla); > + } else { > + return EINVAL; > + } > + } > + > + if (!tc_action->sample.rate || ret) { > + return EINVAL; > + } > + > + *group_id = sgid_alloc_ctx(&sflow_attr); > + if (!*group_id) { > + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action"); > + return ENOENT; > + } > + tc_action->sample.group_id = *group_id; > + flower->action_count++; > + > + return 0; > +} > + > +static int > +parse_userspace_action(struct tc_flower *flower, struct tc_action *tc_action, > + const struct nlattr *userspace_action, > + const struct flow_tnl *tnl, uint32_t *group_id, > + const ovs_u128 *ufid) > +{ > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > + struct dpif_sflow_attr sflow_attr; > + int err; > + > + if (*group_id) { > + VLOG_ERR_RL(&error_rl, "Only a single TC_SAMPLE action per flow is" > + " supported."); > + return EOPNOTSUPP; > + } > + > + /* If sampling rate is 1, there is only a sFlow cookie inside of a > + * userspace action, but no sample attribute. That means we can > + * only offload userspace actions for sFlow. > + */ > + memset(&sflow_attr, 0, sizeof sflow_attr); > + sflow_attr.ufid = *ufid; > + if (flower->tunnel) { > + sflow_attr.tunnel = CONST_CAST(struct flow_tnl *, tnl); > + } > + err = parse_userspace_attributes(userspace_action, &sflow_attr); > + if (err) { > + return err; > + } > + sflow_attr.action = userspace_action; > + *group_id = sgid_alloc_ctx(&sflow_attr); > + if (!*group_id) { > + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action."); Log messages do not seem to end with a dot, so please remove it. > + return ENOENT; > + } > + tc_action->type = TC_ACT_SAMPLE; > + tc_action->sample.group_id = *group_id; > + tc_action->sample.rate = 1; > + flower->action_count++; > + > + return 0; > +} > + > static int > netdev_tc_flow_put(struct netdev *netdev, struct match *match, > struct nlattr *actions, size_t actions_len, > @@ -1805,6 +1964,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > const struct flow_tnl *tnl_mask = &mask->tunnel; > struct tc_action *action; > bool recirc_act = false; > + uint32_t sample_gid = 0; > uint32_t block_id = 0; > struct nlattr *nla; > struct tcf_id id; > @@ -2057,7 +2217,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > NL_ATTR_FOR_EACH(nla, left, actions, actions_len) { > if (flower.action_count >= TCA_ACT_MAX_NUM) { > VLOG_DBG_RL(&rl, "Can only support %d actions", TCA_ACT_MAX_NUM); > - return EOPNOTSUPP; > + err = EOPNOTSUPP; > + goto out; > } > action = &flower.actions[flower.action_count]; > if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { > @@ -2067,7 +2228,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > > if (!outdev) { > VLOG_DBG_RL(&rl, "Can't find netdev for output port %d", port); > - return ENODEV; > + err = ENODEV; > + goto out; > } > action->out.ifindex_out = netdev_get_ifindex(outdev); > action->out.ingress = is_internal_port(netdev_get_type(outdev)); > @@ -2105,7 +2267,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > > err = parse_put_flow_set_action(&flower, action, set, set_len); > if (err) { > - return err; > + goto out; > } > if (action->type == TC_ACT_ENCAP) { > action->encap.tp_dst = info->tp_dst_port; > @@ -2118,7 +2280,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > err = parse_put_flow_set_masked_action(&flower, action, set, > set_len, true); > if (err) { > - return err; > + goto out; > } > } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CT) { > const struct nlattr *ct = nl_attr_get(nla); > @@ -2130,7 +2292,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > > err = parse_put_flow_ct_action(&flower, action, ct, ct_len); > if (err) { > - return err; > + goto out; > } > } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CT_CLEAR) { > action->type = TC_ACT_CT; > @@ -2146,20 +2308,41 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > action->chain = 0; /* 0 is reserved and not used by recirc. */ > flower.action_count++; > } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SAMPLE) { > - struct dpif_sflow_attr sflow_attr; > - > - memset(&sflow_attr, 0, sizeof sflow_attr); > - sgid_alloc_ctx(&sflow_attr); > + if (!info->psample_exists) { Just call dpif_offload_netlink_psample_supported() here, no need to put this in a structure. > + VLOG_DBG_RL(&rl, "unsupported put action type: %d, psample is" > + "not initialized successfully", nl_attr_type(nla)); Capital U for Unsupported. > + err = EOPNOTSUPP; > + goto out; > + } > + err = parse_sample_action(&flower, action, nla, tnl, &sample_gid, > + ufid); > + if (err) { > + goto out; > + } > + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) { > + if (!info->psample_exists) { Just call dpif_offload_netlink_psample_supported() here, no need to put this in a structure. > + VLOG_DBG_RL(&rl, "unsupported put action type: %d, psample is" > + "not initialized successfully", nl_attr_type(nla)); Capital U for Unsupported. > + err = EOPNOTSUPP; > + goto out; > + } > + err = parse_userspace_action(&flower, action, nla, tnl, > + &sample_gid, ufid); > + if (err) { > + goto out; > + } > } else { > VLOG_DBG_RL(&rl, "unsupported put action type: %d", > nl_attr_type(nla)); > - return EOPNOTSUPP; > + err = EOPNOTSUPP; > + goto out; > } > } > > if ((chain || recirc_act) && !info->recirc_id_shared_with_tc) { > VLOG_ERR_RL(&error_rl, "flow_put: recirc_id sharing not supported"); > - return EOPNOTSUPP; > + err = EOPNOTSUPP; > + goto out; > } > > if (get_ufid_tc_mapping(ufid, &id) == 0) { > @@ -2172,20 +2355,29 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > prio = get_prio_for_tc_flower(&flower); > if (prio == 0) { > VLOG_ERR_RL(&rl, "couldn't get tc prio: %s", ovs_strerror(ENOSPC)); > - return ENOSPC; > + err = ENOSPC; > + goto out; > } > > flower.act_cookie.data = ufid; > flower.act_cookie.len = sizeof *ufid; > > block_id = get_block_id_from_netdev(netdev); > - id = tc_make_tcf_id_chain(ifindex, block_id, chain, prio, hook); > + id = tc_make_tcf_id_all(ifindex, block_id, chain, prio, hook, sample_gid); > err = tc_replace_flower(&id, &flower); > - if (!err) { > - if (stats) { > - memset(stats, 0, sizeof *stats); > - } > - add_ufid_tc_mapping(netdev, ufid, &id); > + if (err) { > + goto out; > + } > + > + if (stats) { > + memset(stats, 0, sizeof *stats); > + } > + add_ufid_tc_mapping(netdev, ufid, &id); > + return 0; > + > +out: > + if (sample_gid) { > + sgid_free(sample_gid); > } > > return err; > diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h > index c570b4e62..96d32382f 100644 > --- a/lib/netdev-offload.h > +++ b/lib/netdev-offload.h > @@ -77,6 +77,9 @@ struct offload_info { > bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success > * to delete the original flow. */ > odp_port_t orig_in_port; /* Originating in_port for tnl flows. */ > + bool psample_exists; /* Indicate psample is initialized successfully. > + * Otherwise, return error when offloading sample > + * action. */ See above, this member can be removed. > }; > > int netdev_flow_flush(struct netdev *); > diff --git a/lib/tc.c b/lib/tc.c > index 38a1dfc0e..dc8db6b22 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -23,14 +23,15 @@ > #include <linux/if_packet.h> > #include <linux/rtnetlink.h> > #include <linux/tc_act/tc_csum.h> > +#include <linux/tc_act/tc_ct.h> > #include <linux/tc_act/tc_gact.h> > #include <linux/tc_act/tc_mirred.h> > #include <linux/tc_act/tc_mpls.h> > #include <linux/tc_act/tc_pedit.h> > +#include <linux/tc_act/tc_sample.h> > #include <linux/tc_act/tc_skbedit.h> > #include <linux/tc_act/tc_tunnel_key.h> > #include <linux/tc_act/tc_vlan.h> > -#include <linux/tc_act/tc_ct.h> > #include <linux/gen_stats.h> > #include <net/if.h> > #include <unistd.h> > @@ -1341,6 +1342,38 @@ nl_parse_act_gact(struct nlattr *options, struct tc_flower *flower) > return 0; > } > > +static const struct nl_policy sample_policy[] = { > + [TCA_SAMPLE_PARMS] = { .type = NL_A_UNSPEC, > + .min_len = sizeof(struct tc_sample), > + .optional = false, }, > + [TCA_SAMPLE_PSAMPLE_GROUP] = { .type = NL_A_U32, > + .optional = false, }, > + [TCA_SAMPLE_RATE] = { .type = NL_A_U32, > + .optional = false, }, > +}; > + > +static int > +nl_parse_act_sample(struct nlattr *options, struct tc_flower *flower) > +{ > + struct nlattr *sample_attrs[ARRAY_SIZE(sample_policy)]; > + struct tc_action *action; > + > + if (!nl_parse_nested(options, sample_policy, sample_attrs, > + ARRAY_SIZE(sample_policy))) { > + VLOG_ERR_RL(&error_rl, "failed to parse sample action options"); > + return EPROTO; > + } > + > + action = &flower->actions[flower->action_count++]; > + action->type = TC_ACT_SAMPLE; > + action->sample.group_id = > + nl_attr_get_u32(sample_attrs[TCA_SAMPLE_PSAMPLE_GROUP]); > + action->sample.rate = > + nl_attr_get_u32(sample_attrs[TCA_SAMPLE_RATE]); > + > + return 0; > +} > + > static const struct nl_policy mirred_policy[] = { > [TCA_MIRRED_PARMS] = { .type = NL_A_UNSPEC, > .min_len = sizeof(struct tc_mirred), > @@ -1749,6 +1782,8 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower, > /* Added for TC rule only (not in OvS rule) so ignore. */ > } else if (!strcmp(act_kind, "ct")) { > nl_parse_act_ct(act_options, flower); > + } else if (!strcmp(act_kind, "sample")) { > + nl_parse_act_sample(act_options, flower); > } else { > VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind); > err = EINVAL; > @@ -2375,6 +2410,23 @@ nl_msg_put_act_mirred(struct ofpbuf *request, int ifindex, int action, > nl_msg_end_nested(request, offset); > } > > +static void > +nl_msg_put_act_sample(struct ofpbuf *request, uint32_t rate, uint32_t group_id) > +{ > + size_t offset; > + > + nl_msg_put_string(request, TCA_ACT_KIND, "sample"); > + offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS | NLA_F_NESTED); > + { > + struct tc_sample parm = { .action = TC_ACT_PIPE }; > + > + nl_msg_put_unspec(request, TCA_SAMPLE_PARMS, &parm, sizeof parm); > + nl_msg_put_u32(request, TCA_SAMPLE_RATE, rate); > + nl_msg_put_u32(request, TCA_SAMPLE_PSAMPLE_GROUP, group_id); > + } > + nl_msg_end_nested(request, offset); > +} > + > static inline void > nl_msg_put_act_cookie(struct ofpbuf *request, struct tc_cookie *ck) { > if (ck->len) { > @@ -2634,6 +2686,13 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower) > nl_msg_end_nested(request, act_offset); > } > break; > + case TC_ACT_SAMPLE: { > + act_offset = nl_msg_start_nested(request, act_index++); > + nl_msg_put_act_sample(request, action->sample.rate, > + action->sample.group_id); > + nl_msg_end_nested(request, act_offset); > + } > + break; > case TC_ACT_OUTPUT: { > if (!released && flower->tunnel) { > act_offset = nl_msg_start_nested(request, act_index++); > diff --git a/lib/tc.h b/lib/tc.h > index 2e4084f48..f764d7d1e 100644 > --- a/lib/tc.h > +++ b/lib/tc.h > @@ -174,6 +174,7 @@ enum tc_action_type { > TC_ACT_MPLS_SET, > TC_ACT_GOTO, > TC_ACT_CT, > + TC_ACT_SAMPLE, > }; > > enum nat_type { > @@ -256,6 +257,11 @@ struct tc_action { > bool force; > bool commit; > } ct; > + > + struct { > + uint32_t rate; > + uint32_t group_id; > + } sample; > }; > > enum tc_action_type type; > @@ -294,12 +300,14 @@ tc_make_tcf_id(int ifindex, uint32_t block_id, uint16_t prio, > } > > static inline struct tcf_id > -tc_make_tcf_id_chain(int ifindex, uint32_t block_id, uint32_t chain, > - uint16_t prio, enum tc_qdisc_hook hook) > +tc_make_tcf_id_all(int ifindex, uint32_t block_id, uint32_t chain, > + uint16_t prio, enum tc_qdisc_hook hook, > + uint32_t sample_group_id) > { > struct tcf_id id = tc_make_tcf_id(ifindex, block_id, prio, hook); > > id.chain = chain; > + id.sample_group_id = sample_group_id; > > return id; > } > @@ -313,7 +321,8 @@ is_tcf_id_eq(struct tcf_id *id1, struct tcf_id *id2) > && id1->hook == id2->hook > && id1->block_id == id2->block_id > && id1->ifindex == id2->ifindex > - && id1->chain == id2->chain; > + && id1->chain == id2->chain > + && id1->sample_group_id == id2->sample_group_id; > } > > enum tc_offload_policy { > -- > 2.30.2
On 10/15/2021 8:57 PM, Eelco Chaudron wrote: > See some small comments below... > > On 12 Oct 2021, at 10:19, Chris Mi wrote: > >> Create a unique group ID to map the sFlow info when offloading sFlow >> action to TC. When showing the offloaded datapath flows, translate the >> group ID from TC sample action to sFlow info using the mapping. >> >> Signed-off-by: Chris Mi <cmi@nvidia.com> >> Reviewed-by: Eli Britstein <elibr@nvidia.com> >> --- >> NEWS | 1 + >> lib/dpif-netlink.c | 1 + >> lib/dpif-offload-netlink.c | 6 + >> lib/dpif-offload-provider.h | 1 + >> lib/netdev-offload-tc.c | 228 +++++++++++++++++++++++++++++++++--- >> lib/netdev-offload.h | 3 + >> lib/tc.c | 61 +++++++++- >> lib/tc.h | 15 ++- >> 8 files changed, 294 insertions(+), 22 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index 90f4b1590..d34c87b3c 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -10,6 +10,7 @@ Post-v2.16.0 >> limiting behavior. >> * Add hardware offload support for matching IPv4/IPv6 frag types >> (experimental). >> + - Add sFlow offload support for kernel (netlink) datapath. >> >> >> v2.16.0 - 16 Aug 2021 >> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c >> index 19ae543e6..3f51f319f 100644 >> --- a/lib/dpif-netlink.c >> +++ b/lib/dpif-netlink.c >> @@ -2234,6 +2234,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) >> info.tunnel_csum_on = csum_on; >> info.recirc_id_shared_with_tc = (dpif->user_features >> & OVS_DP_F_TC_RECIRC_SHARING); >> + info.psample_exists = dpif_offload_netlink_psample_exists(); > I would just call dpif_offload_netlink_psample_supported() directly in the netdev_tc_flow_put() API. Done. > >> err = netdev_flow_put(dev, &match, >> CONST_CAST(struct nlattr *, put->actions), >> put->actions_len, >> diff --git a/lib/dpif-offload-netlink.c b/lib/dpif-offload-netlink.c >> index 6ebf1aa0c..4db71ecea 100644 >> --- a/lib/dpif-offload-netlink.c >> +++ b/lib/dpif-offload-netlink.c >> @@ -207,3 +207,9 @@ const struct dpif_offload_api dpif_offload_netlink = { >> .sflow_recv_wait = dpif_offload_netlink_sflow_recv_wait, >> .sflow_recv = dpif_offload_netlink_sflow_recv, >> }; >> + >> +bool >> +dpif_offload_netlink_psample_exists(void) >> +{ >> + return psample_sock != NULL; >> +} > I would call this dpif_offload_netlink_psample_supported() Done. > >> diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h >> index 0a0b71618..573f3add5 100644 >> --- a/lib/dpif-offload-provider.h >> +++ b/lib/dpif-offload-provider.h >> @@ -69,6 +69,7 @@ int dpif_offload_sflow_recv(const struct dpif *dpif, >> struct dpif_offload_sflow *sflow); >> >> #ifdef __linux__ >> +bool dpif_offload_netlink_psample_exists(void); >> extern const struct dpif_offload_api dpif_offload_netlink; >> #endif >> >> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c >> index 204d1c833..a47a8e2c1 100644 >> --- a/lib/netdev-offload-tc.c >> +++ b/lib/netdev-offload-tc.c >> @@ -20,6 +20,7 @@ >> #include <linux/if_ether.h> >> >> #include "dpif.h" >> +#include "dpif-offload-provider.h" >> #include "hash.h" >> #include "openvswitch/hmap.h" >> #include "openvswitch/match.h" >> @@ -1052,6 +1053,19 @@ parse_tc_flower_to_match(struct tc_flower *flower, >> action = flower->actions; >> for (i = 0; i < flower->action_count; i++, action++) { >> switch (action->type) { >> + case TC_ACT_SAMPLE: { >> + const struct sgid_node *node; >> + >> + node = sgid_find(action->sample.group_id); >> + if (!node) { >> + VLOG_WARN("Can't find sample group ID data for ID: %u", >> + action->sample.group_id); >> + return ENOENT; >> + } >> + nl_msg_put(buf, node->sflow.action, >> + node->sflow.action->nla_len); >> + } >> + break; >> case TC_ACT_VLAN_POP: { >> nl_msg_put_flag(buf, OVS_ACTION_ATTR_POP_VLAN); >> } >> @@ -1790,6 +1804,151 @@ parse_match_ct_state_to_flower(struct tc_flower *flower, struct match *match) >> } >> } >> >> +static int >> +parse_userspace_attributes(const struct nlattr *actions, >> + struct dpif_sflow_attr *sflow_attr) >> +{ >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); >> + const struct nlattr *nla; >> + unsigned int left; >> + >> + NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) { >> + if (nl_attr_type(nla) == OVS_USERSPACE_ATTR_USERDATA) { >> + struct user_action_cookie *cookie; >> + >> + cookie = CONST_CAST(struct user_action_cookie *, nl_attr_get(nla)); >> + if (cookie->type == USER_ACTION_COOKIE_SFLOW) { >> + sflow_attr->userdata = nla; >> + return 0; >> + } >> + } >> + } >> + >> + VLOG_DBG_RL(&rl, "Can't offload userspace action other than sFlow."); > Log messages do not seem to end with a dot, so please remove it. Done. > >> + return EOPNOTSUPP; >> +} >> + >> +static int >> +parse_sample_actions_attribute(const struct nlattr *actions, >> + struct dpif_sflow_attr *sflow_attr) >> +{ >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); >> + const struct nlattr *nla; >> + unsigned int left; >> + int err = EINVAL; >> + >> + NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) { >> + if (nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) { >> + err = parse_userspace_attributes(nla, sflow_attr); >> + } else { >> + /* We can't offload other nested actions */ >> + VLOG_DBG_RL(&rl, "Can only offload OVS_ACTION_ATTR_USERSPACE" >> + " attribute."); > Log messages do not seem to end with a dot, so please remove it. Done. > >> + return EINVAL; >> + } >> + } >> + >> + if (err) { >> + VLOG_ERR_RL(&error_rl, "No OVS_ACTION_ATTR_USERSPACE attribute."); > Log messages do not seem to end with a dot, so please remove it. Done. > >> + } >> + return err; >> +} >> + >> +static int >> +parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action, >> + const struct nlattr *sample_action, >> + const struct flow_tnl *tnl, uint32_t *group_id, >> + const ovs_u128 *ufid) >> +{ >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); >> + struct dpif_sflow_attr sflow_attr; >> + const struct nlattr *nla; >> + unsigned int left; >> + int ret = EINVAL; >> + >> + if (*group_id) { >> + VLOG_ERR_RL(&error_rl, " Only a single TC_SAMPLE action per flow is" >> + " supported"); > Please change it to below, so it’s easier to grep for: > > VLOG_ERR_RL(&error_rl, > " Only a single TC_SAMPLE action per flow is supported"); Done. > >> + return EOPNOTSUPP; >> + } >> + >> + memset(&sflow_attr, 0, sizeof sflow_attr); >> + sflow_attr.ufid = *ufid; >> + sflow_attr.action = sample_action; >> + >> + if (flower->tunnel) { >> + sflow_attr.tunnel = CONST_CAST(struct flow_tnl *, tnl); >> + } >> + >> + NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) { >> + if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) { >> + ret = parse_sample_actions_attribute(nla, &sflow_attr); >> + } else if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_PROBABILITY) { >> + tc_action->type = TC_ACT_SAMPLE; >> + tc_action->sample.rate = UINT32_MAX / nl_attr_get_u32(nla); >> + } else { >> + return EINVAL; >> + } >> + } >> + >> + if (!tc_action->sample.rate || ret) { >> + return EINVAL; >> + } >> + >> + *group_id = sgid_alloc_ctx(&sflow_attr); >> + if (!*group_id) { >> + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action"); >> + return ENOENT; >> + } >> + tc_action->sample.group_id = *group_id; >> + flower->action_count++; >> + >> + return 0; >> +} >> + >> +static int >> +parse_userspace_action(struct tc_flower *flower, struct tc_action *tc_action, >> + const struct nlattr *userspace_action, >> + const struct flow_tnl *tnl, uint32_t *group_id, >> + const ovs_u128 *ufid) >> +{ >> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); >> + struct dpif_sflow_attr sflow_attr; >> + int err; >> + >> + if (*group_id) { >> + VLOG_ERR_RL(&error_rl, "Only a single TC_SAMPLE action per flow is" >> + " supported."); >> + return EOPNOTSUPP; >> + } >> + >> + /* If sampling rate is 1, there is only a sFlow cookie inside of a >> + * userspace action, but no sample attribute. That means we can >> + * only offload userspace actions for sFlow. >> + */ >> + memset(&sflow_attr, 0, sizeof sflow_attr); >> + sflow_attr.ufid = *ufid; >> + if (flower->tunnel) { >> + sflow_attr.tunnel = CONST_CAST(struct flow_tnl *, tnl); >> + } >> + err = parse_userspace_attributes(userspace_action, &sflow_attr); >> + if (err) { >> + return err; >> + } >> + sflow_attr.action = userspace_action; >> + *group_id = sgid_alloc_ctx(&sflow_attr); >> + if (!*group_id) { >> + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action."); > Log messages do not seem to end with a dot, so please remove it. Done. > >> + return ENOENT; >> + } >> + tc_action->type = TC_ACT_SAMPLE; >> + tc_action->sample.group_id = *group_id; >> + tc_action->sample.rate = 1; >> + flower->action_count++; >> + >> + return 0; >> +} >> + >> static int >> netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> struct nlattr *actions, size_t actions_len, >> @@ -1805,6 +1964,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> const struct flow_tnl *tnl_mask = &mask->tunnel; >> struct tc_action *action; >> bool recirc_act = false; >> + uint32_t sample_gid = 0; >> uint32_t block_id = 0; >> struct nlattr *nla; >> struct tcf_id id; >> @@ -2057,7 +2217,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> NL_ATTR_FOR_EACH(nla, left, actions, actions_len) { >> if (flower.action_count >= TCA_ACT_MAX_NUM) { >> VLOG_DBG_RL(&rl, "Can only support %d actions", TCA_ACT_MAX_NUM); >> - return EOPNOTSUPP; >> + err = EOPNOTSUPP; >> + goto out; >> } >> action = &flower.actions[flower.action_count]; >> if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { >> @@ -2067,7 +2228,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> >> if (!outdev) { >> VLOG_DBG_RL(&rl, "Can't find netdev for output port %d", port); >> - return ENODEV; >> + err = ENODEV; >> + goto out; >> } >> action->out.ifindex_out = netdev_get_ifindex(outdev); >> action->out.ingress = is_internal_port(netdev_get_type(outdev)); >> @@ -2105,7 +2267,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> >> err = parse_put_flow_set_action(&flower, action, set, set_len); >> if (err) { >> - return err; >> + goto out; >> } >> if (action->type == TC_ACT_ENCAP) { >> action->encap.tp_dst = info->tp_dst_port; >> @@ -2118,7 +2280,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> err = parse_put_flow_set_masked_action(&flower, action, set, >> set_len, true); >> if (err) { >> - return err; >> + goto out; >> } >> } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CT) { >> const struct nlattr *ct = nl_attr_get(nla); >> @@ -2130,7 +2292,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> >> err = parse_put_flow_ct_action(&flower, action, ct, ct_len); >> if (err) { >> - return err; >> + goto out; >> } >> } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CT_CLEAR) { >> action->type = TC_ACT_CT; >> @@ -2146,20 +2308,41 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> action->chain = 0; /* 0 is reserved and not used by recirc. */ >> flower.action_count++; >> } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SAMPLE) { >> - struct dpif_sflow_attr sflow_attr; >> - >> - memset(&sflow_attr, 0, sizeof sflow_attr); >> - sgid_alloc_ctx(&sflow_attr); >> + if (!info->psample_exists) { > Just call dpif_offload_netlink_psample_supported() here, no need to put this in a structure. Done. > >> + VLOG_DBG_RL(&rl, "unsupported put action type: %d, psample is" >> + "not initialized successfully", nl_attr_type(nla)); > Capital U for Unsupported. Done. > >> + err = EOPNOTSUPP; >> + goto out; >> + } >> + err = parse_sample_action(&flower, action, nla, tnl, &sample_gid, >> + ufid); >> + if (err) { >> + goto out; >> + } >> + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) { >> + if (!info->psample_exists) { > Just call dpif_offload_netlink_psample_supported() here, no need to put this in a structure. Done. > >> + VLOG_DBG_RL(&rl, "unsupported put action type: %d, psample is" >> + "not initialized successfully", nl_attr_type(nla)); > Capital U for Unsupported. Done. > >> + err = EOPNOTSUPP; >> + goto out; >> + } >> + err = parse_userspace_action(&flower, action, nla, tnl, >> + &sample_gid, ufid); >> + if (err) { >> + goto out; >> + } >> } else { >> VLOG_DBG_RL(&rl, "unsupported put action type: %d", >> nl_attr_type(nla)); >> - return EOPNOTSUPP; >> + err = EOPNOTSUPP; >> + goto out; >> } >> } >> >> if ((chain || recirc_act) && !info->recirc_id_shared_with_tc) { >> VLOG_ERR_RL(&error_rl, "flow_put: recirc_id sharing not supported"); >> - return EOPNOTSUPP; >> + err = EOPNOTSUPP; >> + goto out; >> } >> >> if (get_ufid_tc_mapping(ufid, &id) == 0) { >> @@ -2172,20 +2355,29 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, >> prio = get_prio_for_tc_flower(&flower); >> if (prio == 0) { >> VLOG_ERR_RL(&rl, "couldn't get tc prio: %s", ovs_strerror(ENOSPC)); >> - return ENOSPC; >> + err = ENOSPC; >> + goto out; >> } >> >> flower.act_cookie.data = ufid; >> flower.act_cookie.len = sizeof *ufid; >> >> block_id = get_block_id_from_netdev(netdev); >> - id = tc_make_tcf_id_chain(ifindex, block_id, chain, prio, hook); >> + id = tc_make_tcf_id_all(ifindex, block_id, chain, prio, hook, sample_gid); >> err = tc_replace_flower(&id, &flower); >> - if (!err) { >> - if (stats) { >> - memset(stats, 0, sizeof *stats); >> - } >> - add_ufid_tc_mapping(netdev, ufid, &id); >> + if (err) { >> + goto out; >> + } >> + >> + if (stats) { >> + memset(stats, 0, sizeof *stats); >> + } >> + add_ufid_tc_mapping(netdev, ufid, &id); >> + return 0; >> + >> +out: >> + if (sample_gid) { >> + sgid_free(sample_gid); >> } >> >> return err; >> diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h >> index c570b4e62..96d32382f 100644 >> --- a/lib/netdev-offload.h >> +++ b/lib/netdev-offload.h >> @@ -77,6 +77,9 @@ struct offload_info { >> bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success >> * to delete the original flow. */ >> odp_port_t orig_in_port; /* Originating in_port for tnl flows. */ >> + bool psample_exists; /* Indicate psample is initialized successfully. >> + * Otherwise, return error when offloading sample >> + * action. */ > See above, this member can be removed. Done. > >> }; >> >> int netdev_flow_flush(struct netdev *); >> diff --git a/lib/tc.c b/lib/tc.c >> index 38a1dfc0e..dc8db6b22 100644 >> --- a/lib/tc.c >> +++ b/lib/tc.c >> @@ -23,14 +23,15 @@ >> #include <linux/if_packet.h> >> #include <linux/rtnetlink.h> >> #include <linux/tc_act/tc_csum.h> >> +#include <linux/tc_act/tc_ct.h> >> #include <linux/tc_act/tc_gact.h> >> #include <linux/tc_act/tc_mirred.h> >> #include <linux/tc_act/tc_mpls.h> >> #include <linux/tc_act/tc_pedit.h> >> +#include <linux/tc_act/tc_sample.h> >> #include <linux/tc_act/tc_skbedit.h> >> #include <linux/tc_act/tc_tunnel_key.h> >> #include <linux/tc_act/tc_vlan.h> >> -#include <linux/tc_act/tc_ct.h> >> #include <linux/gen_stats.h> >> #include <net/if.h> >> #include <unistd.h> >> @@ -1341,6 +1342,38 @@ nl_parse_act_gact(struct nlattr *options, struct tc_flower *flower) >> return 0; >> } >> >> +static const struct nl_policy sample_policy[] = { >> + [TCA_SAMPLE_PARMS] = { .type = NL_A_UNSPEC, >> + .min_len = sizeof(struct tc_sample), >> + .optional = false, }, >> + [TCA_SAMPLE_PSAMPLE_GROUP] = { .type = NL_A_U32, >> + .optional = false, }, >> + [TCA_SAMPLE_RATE] = { .type = NL_A_U32, >> + .optional = false, }, >> +}; >> + >> +static int >> +nl_parse_act_sample(struct nlattr *options, struct tc_flower *flower) >> +{ >> + struct nlattr *sample_attrs[ARRAY_SIZE(sample_policy)]; >> + struct tc_action *action; >> + >> + if (!nl_parse_nested(options, sample_policy, sample_attrs, >> + ARRAY_SIZE(sample_policy))) { >> + VLOG_ERR_RL(&error_rl, "failed to parse sample action options"); >> + return EPROTO; >> + } >> + >> + action = &flower->actions[flower->action_count++]; >> + action->type = TC_ACT_SAMPLE; >> + action->sample.group_id = >> + nl_attr_get_u32(sample_attrs[TCA_SAMPLE_PSAMPLE_GROUP]); >> + action->sample.rate = >> + nl_attr_get_u32(sample_attrs[TCA_SAMPLE_RATE]); >> + >> + return 0; >> +} >> + >> static const struct nl_policy mirred_policy[] = { >> [TCA_MIRRED_PARMS] = { .type = NL_A_UNSPEC, >> .min_len = sizeof(struct tc_mirred), >> @@ -1749,6 +1782,8 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower, >> /* Added for TC rule only (not in OvS rule) so ignore. */ >> } else if (!strcmp(act_kind, "ct")) { >> nl_parse_act_ct(act_options, flower); >> + } else if (!strcmp(act_kind, "sample")) { >> + nl_parse_act_sample(act_options, flower); >> } else { >> VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind); >> err = EINVAL; >> @@ -2375,6 +2410,23 @@ nl_msg_put_act_mirred(struct ofpbuf *request, int ifindex, int action, >> nl_msg_end_nested(request, offset); >> } >> >> +static void >> +nl_msg_put_act_sample(struct ofpbuf *request, uint32_t rate, uint32_t group_id) >> +{ >> + size_t offset; >> + >> + nl_msg_put_string(request, TCA_ACT_KIND, "sample"); >> + offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS | NLA_F_NESTED); >> + { >> + struct tc_sample parm = { .action = TC_ACT_PIPE }; >> + >> + nl_msg_put_unspec(request, TCA_SAMPLE_PARMS, &parm, sizeof parm); >> + nl_msg_put_u32(request, TCA_SAMPLE_RATE, rate); >> + nl_msg_put_u32(request, TCA_SAMPLE_PSAMPLE_GROUP, group_id); >> + } >> + nl_msg_end_nested(request, offset); >> +} >> + >> static inline void >> nl_msg_put_act_cookie(struct ofpbuf *request, struct tc_cookie *ck) { >> if (ck->len) { >> @@ -2634,6 +2686,13 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower) >> nl_msg_end_nested(request, act_offset); >> } >> break; >> + case TC_ACT_SAMPLE: { >> + act_offset = nl_msg_start_nested(request, act_index++); >> + nl_msg_put_act_sample(request, action->sample.rate, >> + action->sample.group_id); >> + nl_msg_end_nested(request, act_offset); >> + } >> + break; >> case TC_ACT_OUTPUT: { >> if (!released && flower->tunnel) { >> act_offset = nl_msg_start_nested(request, act_index++); >> diff --git a/lib/tc.h b/lib/tc.h >> index 2e4084f48..f764d7d1e 100644 >> --- a/lib/tc.h >> +++ b/lib/tc.h >> @@ -174,6 +174,7 @@ enum tc_action_type { >> TC_ACT_MPLS_SET, >> TC_ACT_GOTO, >> TC_ACT_CT, >> + TC_ACT_SAMPLE, >> }; >> >> enum nat_type { >> @@ -256,6 +257,11 @@ struct tc_action { >> bool force; >> bool commit; >> } ct; >> + >> + struct { >> + uint32_t rate; >> + uint32_t group_id; >> + } sample; >> }; >> >> enum tc_action_type type; >> @@ -294,12 +300,14 @@ tc_make_tcf_id(int ifindex, uint32_t block_id, uint16_t prio, >> } >> >> static inline struct tcf_id >> -tc_make_tcf_id_chain(int ifindex, uint32_t block_id, uint32_t chain, >> - uint16_t prio, enum tc_qdisc_hook hook) >> +tc_make_tcf_id_all(int ifindex, uint32_t block_id, uint32_t chain, >> + uint16_t prio, enum tc_qdisc_hook hook, >> + uint32_t sample_group_id) >> { >> struct tcf_id id = tc_make_tcf_id(ifindex, block_id, prio, hook); >> >> id.chain = chain; >> + id.sample_group_id = sample_group_id; >> >> return id; >> } >> @@ -313,7 +321,8 @@ is_tcf_id_eq(struct tcf_id *id1, struct tcf_id *id2) >> && id1->hook == id2->hook >> && id1->block_id == id2->block_id >> && id1->ifindex == id2->ifindex >> - && id1->chain == id2->chain; >> + && id1->chain == id2->chain >> + && id1->sample_group_id == id2->sample_group_id; >> } >> >> enum tc_offload_policy { >> -- >> 2.30.2
diff --git a/NEWS b/NEWS index 90f4b1590..d34c87b3c 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,7 @@ Post-v2.16.0 limiting behavior. * Add hardware offload support for matching IPv4/IPv6 frag types (experimental). + - Add sFlow offload support for kernel (netlink) datapath. v2.16.0 - 16 Aug 2021 diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 19ae543e6..3f51f319f 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -2234,6 +2234,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) info.tunnel_csum_on = csum_on; info.recirc_id_shared_with_tc = (dpif->user_features & OVS_DP_F_TC_RECIRC_SHARING); + info.psample_exists = dpif_offload_netlink_psample_exists(); err = netdev_flow_put(dev, &match, CONST_CAST(struct nlattr *, put->actions), put->actions_len, diff --git a/lib/dpif-offload-netlink.c b/lib/dpif-offload-netlink.c index 6ebf1aa0c..4db71ecea 100644 --- a/lib/dpif-offload-netlink.c +++ b/lib/dpif-offload-netlink.c @@ -207,3 +207,9 @@ const struct dpif_offload_api dpif_offload_netlink = { .sflow_recv_wait = dpif_offload_netlink_sflow_recv_wait, .sflow_recv = dpif_offload_netlink_sflow_recv, }; + +bool +dpif_offload_netlink_psample_exists(void) +{ + return psample_sock != NULL; +} diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h index 0a0b71618..573f3add5 100644 --- a/lib/dpif-offload-provider.h +++ b/lib/dpif-offload-provider.h @@ -69,6 +69,7 @@ int dpif_offload_sflow_recv(const struct dpif *dpif, struct dpif_offload_sflow *sflow); #ifdef __linux__ +bool dpif_offload_netlink_psample_exists(void); extern const struct dpif_offload_api dpif_offload_netlink; #endif diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 204d1c833..a47a8e2c1 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -20,6 +20,7 @@ #include <linux/if_ether.h> #include "dpif.h" +#include "dpif-offload-provider.h" #include "hash.h" #include "openvswitch/hmap.h" #include "openvswitch/match.h" @@ -1052,6 +1053,19 @@ parse_tc_flower_to_match(struct tc_flower *flower, action = flower->actions; for (i = 0; i < flower->action_count; i++, action++) { switch (action->type) { + case TC_ACT_SAMPLE: { + const struct sgid_node *node; + + node = sgid_find(action->sample.group_id); + if (!node) { + VLOG_WARN("Can't find sample group ID data for ID: %u", + action->sample.group_id); + return ENOENT; + } + nl_msg_put(buf, node->sflow.action, + node->sflow.action->nla_len); + } + break; case TC_ACT_VLAN_POP: { nl_msg_put_flag(buf, OVS_ACTION_ATTR_POP_VLAN); } @@ -1790,6 +1804,151 @@ parse_match_ct_state_to_flower(struct tc_flower *flower, struct match *match) } } +static int +parse_userspace_attributes(const struct nlattr *actions, + struct dpif_sflow_attr *sflow_attr) +{ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); + const struct nlattr *nla; + unsigned int left; + + NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) { + if (nl_attr_type(nla) == OVS_USERSPACE_ATTR_USERDATA) { + struct user_action_cookie *cookie; + + cookie = CONST_CAST(struct user_action_cookie *, nl_attr_get(nla)); + if (cookie->type == USER_ACTION_COOKIE_SFLOW) { + sflow_attr->userdata = nla; + return 0; + } + } + } + + VLOG_DBG_RL(&rl, "Can't offload userspace action other than sFlow."); + return EOPNOTSUPP; +} + +static int +parse_sample_actions_attribute(const struct nlattr *actions, + struct dpif_sflow_attr *sflow_attr) +{ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); + const struct nlattr *nla; + unsigned int left; + int err = EINVAL; + + NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) { + if (nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) { + err = parse_userspace_attributes(nla, sflow_attr); + } else { + /* We can't offload other nested actions */ + VLOG_DBG_RL(&rl, "Can only offload OVS_ACTION_ATTR_USERSPACE" + " attribute."); + return EINVAL; + } + } + + if (err) { + VLOG_ERR_RL(&error_rl, "No OVS_ACTION_ATTR_USERSPACE attribute."); + } + return err; +} + +static int +parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action, + const struct nlattr *sample_action, + const struct flow_tnl *tnl, uint32_t *group_id, + const ovs_u128 *ufid) +{ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); + struct dpif_sflow_attr sflow_attr; + const struct nlattr *nla; + unsigned int left; + int ret = EINVAL; + + if (*group_id) { + VLOG_ERR_RL(&error_rl, " Only a single TC_SAMPLE action per flow is" + " supported"); + return EOPNOTSUPP; + } + + memset(&sflow_attr, 0, sizeof sflow_attr); + sflow_attr.ufid = *ufid; + sflow_attr.action = sample_action; + + if (flower->tunnel) { + sflow_attr.tunnel = CONST_CAST(struct flow_tnl *, tnl); + } + + NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) { + if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) { + ret = parse_sample_actions_attribute(nla, &sflow_attr); + } else if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_PROBABILITY) { + tc_action->type = TC_ACT_SAMPLE; + tc_action->sample.rate = UINT32_MAX / nl_attr_get_u32(nla); + } else { + return EINVAL; + } + } + + if (!tc_action->sample.rate || ret) { + return EINVAL; + } + + *group_id = sgid_alloc_ctx(&sflow_attr); + if (!*group_id) { + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action"); + return ENOENT; + } + tc_action->sample.group_id = *group_id; + flower->action_count++; + + return 0; +} + +static int +parse_userspace_action(struct tc_flower *flower, struct tc_action *tc_action, + const struct nlattr *userspace_action, + const struct flow_tnl *tnl, uint32_t *group_id, + const ovs_u128 *ufid) +{ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); + struct dpif_sflow_attr sflow_attr; + int err; + + if (*group_id) { + VLOG_ERR_RL(&error_rl, "Only a single TC_SAMPLE action per flow is" + " supported."); + return EOPNOTSUPP; + } + + /* If sampling rate is 1, there is only a sFlow cookie inside of a + * userspace action, but no sample attribute. That means we can + * only offload userspace actions for sFlow. + */ + memset(&sflow_attr, 0, sizeof sflow_attr); + sflow_attr.ufid = *ufid; + if (flower->tunnel) { + sflow_attr.tunnel = CONST_CAST(struct flow_tnl *, tnl); + } + err = parse_userspace_attributes(userspace_action, &sflow_attr); + if (err) { + return err; + } + sflow_attr.action = userspace_action; + *group_id = sgid_alloc_ctx(&sflow_attr); + if (!*group_id) { + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action."); + return ENOENT; + } + tc_action->type = TC_ACT_SAMPLE; + tc_action->sample.group_id = *group_id; + tc_action->sample.rate = 1; + flower->action_count++; + + return 0; +} + static int netdev_tc_flow_put(struct netdev *netdev, struct match *match, struct nlattr *actions, size_t actions_len, @@ -1805,6 +1964,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, const struct flow_tnl *tnl_mask = &mask->tunnel; struct tc_action *action; bool recirc_act = false; + uint32_t sample_gid = 0; uint32_t block_id = 0; struct nlattr *nla; struct tcf_id id; @@ -2057,7 +2217,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, NL_ATTR_FOR_EACH(nla, left, actions, actions_len) { if (flower.action_count >= TCA_ACT_MAX_NUM) { VLOG_DBG_RL(&rl, "Can only support %d actions", TCA_ACT_MAX_NUM); - return EOPNOTSUPP; + err = EOPNOTSUPP; + goto out; } action = &flower.actions[flower.action_count]; if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { @@ -2067,7 +2228,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, if (!outdev) { VLOG_DBG_RL(&rl, "Can't find netdev for output port %d", port); - return ENODEV; + err = ENODEV; + goto out; } action->out.ifindex_out = netdev_get_ifindex(outdev); action->out.ingress = is_internal_port(netdev_get_type(outdev)); @@ -2105,7 +2267,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, err = parse_put_flow_set_action(&flower, action, set, set_len); if (err) { - return err; + goto out; } if (action->type == TC_ACT_ENCAP) { action->encap.tp_dst = info->tp_dst_port; @@ -2118,7 +2280,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, err = parse_put_flow_set_masked_action(&flower, action, set, set_len, true); if (err) { - return err; + goto out; } } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CT) { const struct nlattr *ct = nl_attr_get(nla); @@ -2130,7 +2292,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, err = parse_put_flow_ct_action(&flower, action, ct, ct_len); if (err) { - return err; + goto out; } } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CT_CLEAR) { action->type = TC_ACT_CT; @@ -2146,20 +2308,41 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, action->chain = 0; /* 0 is reserved and not used by recirc. */ flower.action_count++; } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SAMPLE) { - struct dpif_sflow_attr sflow_attr; - - memset(&sflow_attr, 0, sizeof sflow_attr); - sgid_alloc_ctx(&sflow_attr); + if (!info->psample_exists) { + VLOG_DBG_RL(&rl, "unsupported put action type: %d, psample is" + "not initialized successfully", nl_attr_type(nla)); + err = EOPNOTSUPP; + goto out; + } + err = parse_sample_action(&flower, action, nla, tnl, &sample_gid, + ufid); + if (err) { + goto out; + } + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) { + if (!info->psample_exists) { + VLOG_DBG_RL(&rl, "unsupported put action type: %d, psample is" + "not initialized successfully", nl_attr_type(nla)); + err = EOPNOTSUPP; + goto out; + } + err = parse_userspace_action(&flower, action, nla, tnl, + &sample_gid, ufid); + if (err) { + goto out; + } } else { VLOG_DBG_RL(&rl, "unsupported put action type: %d", nl_attr_type(nla)); - return EOPNOTSUPP; + err = EOPNOTSUPP; + goto out; } } if ((chain || recirc_act) && !info->recirc_id_shared_with_tc) { VLOG_ERR_RL(&error_rl, "flow_put: recirc_id sharing not supported"); - return EOPNOTSUPP; + err = EOPNOTSUPP; + goto out; } if (get_ufid_tc_mapping(ufid, &id) == 0) { @@ -2172,20 +2355,29 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, prio = get_prio_for_tc_flower(&flower); if (prio == 0) { VLOG_ERR_RL(&rl, "couldn't get tc prio: %s", ovs_strerror(ENOSPC)); - return ENOSPC; + err = ENOSPC; + goto out; } flower.act_cookie.data = ufid; flower.act_cookie.len = sizeof *ufid; block_id = get_block_id_from_netdev(netdev); - id = tc_make_tcf_id_chain(ifindex, block_id, chain, prio, hook); + id = tc_make_tcf_id_all(ifindex, block_id, chain, prio, hook, sample_gid); err = tc_replace_flower(&id, &flower); - if (!err) { - if (stats) { - memset(stats, 0, sizeof *stats); - } - add_ufid_tc_mapping(netdev, ufid, &id); + if (err) { + goto out; + } + + if (stats) { + memset(stats, 0, sizeof *stats); + } + add_ufid_tc_mapping(netdev, ufid, &id); + return 0; + +out: + if (sample_gid) { + sgid_free(sample_gid); } return err; diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h index c570b4e62..96d32382f 100644 --- a/lib/netdev-offload.h +++ b/lib/netdev-offload.h @@ -77,6 +77,9 @@ struct offload_info { bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success * to delete the original flow. */ odp_port_t orig_in_port; /* Originating in_port for tnl flows. */ + bool psample_exists; /* Indicate psample is initialized successfully. + * Otherwise, return error when offloading sample + * action. */ }; int netdev_flow_flush(struct netdev *); diff --git a/lib/tc.c b/lib/tc.c index 38a1dfc0e..dc8db6b22 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -23,14 +23,15 @@ #include <linux/if_packet.h> #include <linux/rtnetlink.h> #include <linux/tc_act/tc_csum.h> +#include <linux/tc_act/tc_ct.h> #include <linux/tc_act/tc_gact.h> #include <linux/tc_act/tc_mirred.h> #include <linux/tc_act/tc_mpls.h> #include <linux/tc_act/tc_pedit.h> +#include <linux/tc_act/tc_sample.h> #include <linux/tc_act/tc_skbedit.h> #include <linux/tc_act/tc_tunnel_key.h> #include <linux/tc_act/tc_vlan.h> -#include <linux/tc_act/tc_ct.h> #include <linux/gen_stats.h> #include <net/if.h> #include <unistd.h> @@ -1341,6 +1342,38 @@ nl_parse_act_gact(struct nlattr *options, struct tc_flower *flower) return 0; } +static const struct nl_policy sample_policy[] = { + [TCA_SAMPLE_PARMS] = { .type = NL_A_UNSPEC, + .min_len = sizeof(struct tc_sample), + .optional = false, }, + [TCA_SAMPLE_PSAMPLE_GROUP] = { .type = NL_A_U32, + .optional = false, }, + [TCA_SAMPLE_RATE] = { .type = NL_A_U32, + .optional = false, }, +}; + +static int +nl_parse_act_sample(struct nlattr *options, struct tc_flower *flower) +{ + struct nlattr *sample_attrs[ARRAY_SIZE(sample_policy)]; + struct tc_action *action; + + if (!nl_parse_nested(options, sample_policy, sample_attrs, + ARRAY_SIZE(sample_policy))) { + VLOG_ERR_RL(&error_rl, "failed to parse sample action options"); + return EPROTO; + } + + action = &flower->actions[flower->action_count++]; + action->type = TC_ACT_SAMPLE; + action->sample.group_id = + nl_attr_get_u32(sample_attrs[TCA_SAMPLE_PSAMPLE_GROUP]); + action->sample.rate = + nl_attr_get_u32(sample_attrs[TCA_SAMPLE_RATE]); + + return 0; +} + static const struct nl_policy mirred_policy[] = { [TCA_MIRRED_PARMS] = { .type = NL_A_UNSPEC, .min_len = sizeof(struct tc_mirred), @@ -1749,6 +1782,8 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower, /* Added for TC rule only (not in OvS rule) so ignore. */ } else if (!strcmp(act_kind, "ct")) { nl_parse_act_ct(act_options, flower); + } else if (!strcmp(act_kind, "sample")) { + nl_parse_act_sample(act_options, flower); } else { VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind); err = EINVAL; @@ -2375,6 +2410,23 @@ nl_msg_put_act_mirred(struct ofpbuf *request, int ifindex, int action, nl_msg_end_nested(request, offset); } +static void +nl_msg_put_act_sample(struct ofpbuf *request, uint32_t rate, uint32_t group_id) +{ + size_t offset; + + nl_msg_put_string(request, TCA_ACT_KIND, "sample"); + offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS | NLA_F_NESTED); + { + struct tc_sample parm = { .action = TC_ACT_PIPE }; + + nl_msg_put_unspec(request, TCA_SAMPLE_PARMS, &parm, sizeof parm); + nl_msg_put_u32(request, TCA_SAMPLE_RATE, rate); + nl_msg_put_u32(request, TCA_SAMPLE_PSAMPLE_GROUP, group_id); + } + nl_msg_end_nested(request, offset); +} + static inline void nl_msg_put_act_cookie(struct ofpbuf *request, struct tc_cookie *ck) { if (ck->len) { @@ -2634,6 +2686,13 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower) nl_msg_end_nested(request, act_offset); } break; + case TC_ACT_SAMPLE: { + act_offset = nl_msg_start_nested(request, act_index++); + nl_msg_put_act_sample(request, action->sample.rate, + action->sample.group_id); + nl_msg_end_nested(request, act_offset); + } + break; case TC_ACT_OUTPUT: { if (!released && flower->tunnel) { act_offset = nl_msg_start_nested(request, act_index++); diff --git a/lib/tc.h b/lib/tc.h index 2e4084f48..f764d7d1e 100644 --- a/lib/tc.h +++ b/lib/tc.h @@ -174,6 +174,7 @@ enum tc_action_type { TC_ACT_MPLS_SET, TC_ACT_GOTO, TC_ACT_CT, + TC_ACT_SAMPLE, }; enum nat_type { @@ -256,6 +257,11 @@ struct tc_action { bool force; bool commit; } ct; + + struct { + uint32_t rate; + uint32_t group_id; + } sample; }; enum tc_action_type type; @@ -294,12 +300,14 @@ tc_make_tcf_id(int ifindex, uint32_t block_id, uint16_t prio, } static inline struct tcf_id -tc_make_tcf_id_chain(int ifindex, uint32_t block_id, uint32_t chain, - uint16_t prio, enum tc_qdisc_hook hook) +tc_make_tcf_id_all(int ifindex, uint32_t block_id, uint32_t chain, + uint16_t prio, enum tc_qdisc_hook hook, + uint32_t sample_group_id) { struct tcf_id id = tc_make_tcf_id(ifindex, block_id, prio, hook); id.chain = chain; + id.sample_group_id = sample_group_id; return id; } @@ -313,7 +321,8 @@ is_tcf_id_eq(struct tcf_id *id1, struct tcf_id *id2) && id1->hook == id2->hook && id1->block_id == id2->block_id && id1->ifindex == id2->ifindex - && id1->chain == id2->chain; + && id1->chain == id2->chain + && id1->sample_group_id == id2->sample_group_id; } enum tc_offload_policy {