On 19 Jun 2023, at 7:01, Chris Mi wrote:
> <SNIP>
>>> + if (err) {
>>> + VLOG_ERR_RL(&error_rl, "No OVS_ACTION_ATTR_USERSPACE attribute");
>>> + }
>>> + return err;
>>> +}
>>> +
>>> +static void
>>> +offload_sample_init(struct offload_sample *sample,
>>> + const struct nlattr *next_actions,
>>> + size_t next_actions_len,
>>> + struct tc_flower *flower,
>>> + const struct flow_tnl *tnl)
>>> +{
>>> + memset(sample, 0, sizeof *sample);
>>> + if (flower->tunnel) {
>>> + sample->tunnel = CONST_CAST(struct flow_tnl *, tnl);
>>> + }
>>> + sample->userspace_actions = CONST_CAST(struct nlattr *, next_actions);
>>> + sample->userspace_actions_len = next_actions_len;
>> Here we should initialize the userspace_actions, to make it work :)
>> This was previously in the clone action in patch 3:
>>
>> @@ -2148,8 +2148,11 @@ offload_sample_init(struct offload_sample *sample,
>> if (flower->tunnel) {
>> sample->tunnel = CONST_CAST(struct flow_tnl *, tnl);
>> }
>> - sample->userspace_actions = CONST_CAST(struct nlattr *, next_actions);
>> - sample->userspace_actions_len = next_actions_len;
>> +
>> + sample->userspace_actions = xmalloc(next_actions_len + NLA_HDRLEN);
>> + nullable_memcpy((char *) sample->userspace_actions + NLA_HDRLEN,
>> + next_actions, next_actions_len);
>> + sample->userspace_actions->nla_len = next_actions_len + NLA_HDRLEN;
>> sample->ifindex = flower->ifindex;
>>
>> One remaining question is, should we set the nla_type here to
>> OVS_ACTION_ATTR_SAMPLE or OVS_ACTION_ATTR_USERSPACE as this what they are,
>> or is it safe to not set this as the upcall handling will ignore the type?
> According to function dpif_get_actions(), nla_type is ignored:
>
> if (upcall->actions) {
> /* Actions were passed up from datapath. */
> *actions = nl_attr_get(upcall->actions);
> actions_len = nl_attr_get_size(upcall->actions);
> }
Thanks! I guess I could have done this myself :(
Cheers,
Eelco
Reviewing your v28 right now. And doing some final testing!
>>
>>> + sample->ifindex = flower->ifindex;
>>> +}
>>> +
>>> +static int
>>> +parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action,
>>> + const struct nlattr *next_actions, size_t
>>> next_actions_len,
>>> + const struct nlattr *sample_action,
>>> + const struct flow_tnl *tnl)
>>> +{
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>>> + struct offload_sample sample;
>>> + const struct nlattr *nla;
>>> + unsigned int left;
>>> + uint32_t sgid;
>>> + int err;
>> As you change the function, you should set err to EINVAL here :)
>>
>>> +
>>> + offload_sample_init(&sample, next_actions, next_actions_len, flower,
>>> tnl);
>>> + sample.action = CONST_CAST(struct nlattr *, sample_action);
>>> +
>>> + err = EINVAL;
>> Remove this here, as you no longer override it with offload_sample_init()
>>
>>> + NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) {
>>> + if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) {
>>> + err = parse_sample_actions_attribute(nla, &sample);
>>> + if (err) {
>>> + break;
>>> + }
>>> + } 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);
>>> + if (!tc_action->sample.rate) {
>>> + break;
>>> + }
>>> + } else {
>> Here you removed err = EINVAL; but it should be here if you want to error
>> out on unsupported attributes.
>>
>> I re-wrote the function a bit, to clean it up, and made it look a bit more
>> like the v18 version. I removed the conditional breaks in the if() branches,
>> as it made it looks like we check for errors in these branch, but in fact
>> the checks are there to verify the attributes where present.
>> Also made sure we do not trash the tc_action structure on failure.
>>
>> static int
>> parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action,
>> const struct nlattr *next_actions, size_t
>> next_actions_len,
>> const struct nlattr *sample_action,
>> const struct flow_tnl *tnl)
>> {
>> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>> struct offload_sample sample;
>> const struct nlattr *nla;
>> unsigned int left;
>> uint32_t rate = 0;
>> int ret = EINVAL;
>> uint32_t sgid;
>>
>> offload_sample_init(&sample, next_actions, next_actions_len, flower,
>> tnl);
>> sample.action = CONST_CAST(struct nlattr *, sample_action);
>>
>> NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) {
>> if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) {
>> ret = parse_sample_actions_attribute(nla, &sample);
>> } else if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_PROBABILITY) {
>> rate = UINT32_MAX / nl_attr_get_u32(nla);
>> } else {
>> return EINVAL;
>> }
>> }
>>
>> /* This check makes sure both attributes above were present and valid.
>> */
>> if (!rate || ret) {
>> return EINVAL;
>> }
>>
>> sgid = sgid_alloc(&sample);
>> if (!sgid) {
>> VLOG_DBG_RL(&rl, "Failed allocating group id for sample action");
>> return ENOENT;
>> }
>>
>> tc_action->type = TC_ACT_SAMPLE;
>> tc_action->sample.rate = rate;
>> tc_action->sample.group_id = sgid;
>> flower->action_count++;
>>
>> return 0;
>> }
>>
>>> + break;
>>> + }
>>> + }
>>> +
>>> + if (!tc_action->sample.rate || err) {
>>> + return EINVAL;
>>> + }
>>> +
>>> + sgid = sgid_alloc(&sample);
>>> + if (!sgid) {
>>> + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action");
>>> + return ENOENT;
>>> + }
>>> + tc_action->sample.group_id = sgid;
>>> + flower->action_count++;
>>> +
>>> + return err;
>>> +}
>>> +
>>> +static int
>>> +parse_userspace_action(struct tc_flower *flower, struct tc_action
>>> *tc_action,
>>> + const struct nlattr *next_actions,
>>> + size_t next_actions_len,
>>> + const struct nlattr *userspace_action,
>>> + const struct flow_tnl *tnl)
>>> +{
>>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>>> + struct offload_sample sample;
>>> + uint32_t sgid;
>>> + int err;
>>> +
>>> + offload_sample_init(&sample, next_actions, next_actions_len, flower,
>>> tnl);
>>> +
>>> + /* 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. */
>>> + err = parse_userspace_attributes(userspace_action, &sample);
>>> + if (err) {
>>> + return err;
>>> + }
>>> + sample.action = (struct nlattr *) userspace_action;
>>> + sgid = sgid_alloc(&sample);
>>> + if (!sgid) {
>>> + VLOG_DBG_RL(&rl, "Failed allocating group id for sample action");
>>> + return ENOENT;
>>> + }
>>> + tc_action->type = TC_ACT_SAMPLE;
>>> + tc_action->sample.group_id = sgid;
>>> + tc_action->sample.rate = 1;
>>> + flower->action_count++;
>>> +
>>> + return err;
>> Here we can just return 0.
>>
>>> +}
>>>
>>> static int
>>> parse_check_pkt_len_action(struct netdev *netdev, struct tc_flower
>>> *flower,
>>> - struct offload_info *info, struct tc_action
>>> *action,
>>> - const struct nlattr *nla, bool last_action,
>>> + struct offload_info *info,
>>> + const struct flow_tnl *tnl,
>>> + struct tc_action *action, const struct nlattr
>>> *nla,
>>> + bool last_action,
>>> struct tc_action **need_jump_update,
>>> bool *recirc_act)
>>> {
>>> @@ -2070,7 +2283,7 @@ parse_check_pkt_len_action(struct netdev *netdev,
>>> struct tc_flower *flower,
>>> * NOTE: The last_action parameter means that there are no more
>>> actions
>>> * after the if () then ... else () case. */
>>> nl_actions = a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER];
>>> - err = netdev_tc_parse_nl_actions(netdev, flower, info,
>>> + err = netdev_tc_parse_nl_actions(netdev, flower, info, tnl,
>>> nl_attr_get(nl_actions),
>>> nl_attr_get_size(nl_actions),
>>> recirc_act, !last_action,
>>> @@ -2086,7 +2299,7 @@ parse_check_pkt_len_action(struct netdev *netdev,
>>> struct tc_flower *flower,
>>>
>>> /* Parse and add the less than action(s). */
>>> nl_actions = a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL];
>>> - err = netdev_tc_parse_nl_actions(netdev, flower, info,
>>> + err = netdev_tc_parse_nl_actions(netdev, flower, info, tnl,
>>> nl_attr_get(nl_actions),
>>> nl_attr_get_size(nl_actions),
>>> recirc_act, !last_action,
>>> @@ -2139,6 +2352,7 @@ parse_check_pkt_len_action(struct netdev *netdev,
>>> struct tc_flower *flower,
>>> static int
>>> netdev_tc_parse_nl_actions(struct netdev *netdev, struct tc_flower
>>> *flower,
>>> struct offload_info *info,
>>> + const struct flow_tnl *tnl,
>>> const struct nlattr *actions, size_t
>>> actions_len,
>>> bool *recirc_act, bool more_actions,
>>> struct tc_action **need_jump_update)
>>> @@ -2268,7 +2482,8 @@ netdev_tc_parse_nl_actions(struct netdev *netdev,
>>> struct tc_flower *flower,
>>> action->police.index = police_index;
>>> flower->action_count++;
>>> } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CHECK_PKT_LEN) {
>>> - err = parse_check_pkt_len_action(netdev, flower, info, action,
>>> nla,
>>> + err = parse_check_pkt_len_action(netdev, flower, info, tnl,
>>> + action, nla,
>>> nl_attr_len_pad(nla,
>>> left) >= left
>>> && !more_actions,
>>> @@ -2277,14 +2492,28 @@ netdev_tc_parse_nl_actions(struct netdev *netdev,
>>> struct tc_flower *flower,
>>> if (err) {
>>> return err;
>>> }
>>> - } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SAMPLE) {
>>> - struct offload_sample sample;
>>> - uint32_t sgid;
>>> -
>>> - memset(&sample, 0, sizeof sample);
>>> - sgid = sgid_alloc(&sample);
>>> - sgid_free(sgid);
>>> - return EOPNOTSUPP;
>>> + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SAMPLE ||
>>> + nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) {
>>> + if (!psample_supported()) {
>>> + VLOG_DBG_RL(&rl, "Unsupported put action type: %d, psample
>>> is "
>>> + "not initialized successfully",
>>> nl_attr_type(nla));
>>> + return EOPNOTSUPP;
>>> + }
>>> + if (get_flower_sgid_count(flower) >= MAX_TC_SAMPLES_PER_FLOW) {
>>> + VLOG_ERR_RL(&error_rl, "Only %u TC_SAMPLE action per "
>>> + "flow is supported", MAX_TC_SAMPLES_PER_FLOW);
>>> + return EOPNOTSUPP;
>>> + }
>>> + if (nl_attr_type(nla) == OVS_ACTION_ATTR_SAMPLE) {
>>> + err = parse_sample_action(flower, action, actions,
>>> actions_len,
>>> + nla, tnl);
>>> + } else {
>>> + err = parse_userspace_action(flower, action, actions,
>>> + actions_len, nla, tnl);
>>> + }
>>> + if (err) {
>>> + return err;
>>> + }
>>> } else {
>>> VLOG_DBG_RL(&rl, "unsupported put action type: %d",
>>> nl_attr_type(nla));
>>> @@ -2324,6 +2553,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct
>>> match *match,
>>> }
>>>
>>> memset(&flower, 0, sizeof flower);
>>> + flower.ifindex = ifindex;
>>>
>>> chain = key->recirc_id;
>>> mask->recirc_id = 0;
>>> @@ -2589,16 +2819,17 @@ netdev_tc_flow_put(struct netdev *netdev, struct
>>> match *match,
>>> }
>>>
>>> /* Parse all (nested) actions. */
>>> - err = netdev_tc_parse_nl_actions(netdev, &flower, info,
>>> + err = netdev_tc_parse_nl_actions(netdev, &flower, info, tnl,
>>> actions, actions_len, &recirc_act,
>>> false, NULL);
>>> if (err) {
>>> - return err;
>>> + goto error_out;
>>> }
>>>
>>> if ((chain || recirc_act) && !info->recirc_id_shared_with_tc) {
>>> VLOG_DBG_RL(&rl, "flow_put: recirc_id sharing not supported");
>>> - return EOPNOTSUPP;
>>> + err = EOPNOTSUPP;
>>> + goto error_out;
>>> }
>>>
>>> memset(&adjust_stats, 0, sizeof adjust_stats);
>>> @@ -2612,7 +2843,8 @@ 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 error_out;
>>> }
>>>
>>> flower.act_cookie.data = ufid;
>>> @@ -2621,14 +2853,20 @@ netdev_tc_flow_put(struct netdev *netdev, struct
>>> match *match,
>>> block_id = get_block_id_from_netdev(netdev);
>>> id = tc_make_tcf_id_chain(ifindex, block_id, chain, prio, hook);
>>> err = tc_replace_flower(&id, &flower);
>>> - if (!err) {
>>> - if (stats) {
>>> - memset(stats, 0, sizeof *stats);
>>> - netdev_tc_adjust_stats(stats, &adjust_stats);
>>> - }
>>> - add_ufid_tc_mapping(netdev, ufid, &id, &adjust_stats);
>>> + if (err) {
>>> + goto error_out;
>>> }
>>>
>>> + if (stats) {
>>> + memset(stats, 0, sizeof *stats);
>>> + netdev_tc_adjust_stats(stats, &adjust_stats);
>>> + }
>>> +
>>> + add_ufid_tc_mapping(netdev, ufid, &id, &adjust_stats, &flower);
>>> + return 0;
>>> +
>>> +error_out:
>>> + free_all_flower_sgids(&flower);
>>> return err;
>>> }
>>>
>>> diff --git a/lib/tc.c b/lib/tc.c
>>> index 5c32c6f97..28ca6325a 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>
>>> @@ -1484,6 +1485,38 @@ nl_parse_act_police(const 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),
>>> @@ -1999,6 +2032,8 @@ nl_parse_single_action(struct nlattr *action, struct
>>> tc_flower *flower,
>>> nl_parse_act_ct(act_options, flower);
>>> } else if (!strcmp(act_kind, "police")) {
>>> nl_parse_act_police(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;
>>> @@ -2791,6 +2826,24 @@ 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,
>>> + uint32_t action_pc)
>>> +{
>>> + 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 = action_pc };
>>> +
>>> + 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) {
>>> @@ -3103,6 +3156,7 @@ get_action_index_for_tc_actions(struct tc_flower
>>> *flower, uint16_t act_index,
>>> case TC_ACT_MPLS_SET:
>>> case TC_ACT_GOTO:
>>> case TC_ACT_CT:
>>> + case TC_ACT_SAMPLE:
>>> /* Increase act_index by one if we are sure this type of
>>> action
>>> * will only add one tc action in the kernel. */
>>> act_index++;
>>> @@ -3310,6 +3364,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, action_pc);
>>> + nl_msg_end_nested(request, act_offset);
>>> + }
>>> + break;
>>> case TC_ACT_OUTPUT: {
>>> if (!released && flower->tunnel) {
>>> nl_msg_put_flower_acts_release(request, act_index++);
>>> diff --git a/lib/tc.h b/lib/tc.h
>>> index cdd3b4f60..5f6e15d5c 100644
>>> --- a/lib/tc.h
>>> +++ b/lib/tc.h
>>> @@ -192,6 +192,7 @@ enum tc_action_type {
>>> TC_ACT_CT,
>>> TC_ACT_POLICE,
>>> TC_ACT_POLICE_MTU,
>>> + TC_ACT_SAMPLE,
>>> };
>>>
>>> enum nat_type {
>>> @@ -283,6 +284,10 @@ struct tc_action {
>>> uint32_t result_jump;
>>> uint16_t mtu;
>>> } police;
>>> + struct {
>>> + uint32_t rate;
>>> + uint32_t group_id;
>>> + } sample;
>>> };
>>>
>>> enum tc_action_type type;
>>> @@ -380,6 +385,7 @@ struct tc_flower {
>>> enum tc_offloaded_state offloaded_state;
>>> /* Used to force skip_hw when probing tc features. */
>>> enum tc_offload_policy tc_policy;
>>> + uint16_t ifindex;
>>> };
>>>
>>> int tc_replace_flower(struct tcf_id *id, struct tc_flower *flower);
>>> --
>>> 2.26.3
>> Here is a full diff of the suggested changes, if you sent a v28 with only
>> these changes I can ack the patch (need an answer on one question):
>>
>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>> index 14a4cb393..71ec8ef1f 100644
>> --- a/lib/netdev-offload-tc.c
>> +++ b/lib/netdev-offload-tc.c
>> @@ -2146,8 +2146,11 @@ offload_sample_init(struct offload_sample *sample,
>> if (flower->tunnel) {
>> sample->tunnel = CONST_CAST(struct flow_tnl *, tnl);
>> }
>> - sample->userspace_actions = CONST_CAST(struct nlattr *, next_actions);
>> - sample->userspace_actions_len = next_actions_len;
>> +
>> + sample->userspace_actions = xmalloc(next_actions_len + NLA_HDRLEN);
>> + nullable_memcpy((char *) sample->userspace_actions + NLA_HDRLEN,
>> + next_actions, next_actions_len);
>> + sample->userspace_actions->nla_len = next_actions_len + NLA_HDRLEN;
>> sample->ifindex = flower->ifindex;
>> }
>>
>> @@ -2161,31 +2164,25 @@ parse_sample_action(struct tc_flower *flower, struct
>> tc_action *tc_action,
>> struct offload_sample sample;
>> const struct nlattr *nla;
>> unsigned int left;
>> + uint32_t rate = 0;
>> + int ret = EINVAL;
>> uint32_t sgid;
>> - int err;
>>
>> offload_sample_init(&sample, next_actions, next_actions_len, flower,
>> tnl);
>> sample.action = CONST_CAST(struct nlattr *, sample_action);
>>
>> - err = EINVAL;
>> NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) {
>> if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) {
>> - err = parse_sample_actions_attribute(nla, &sample);
>> - if (err) {
>> - break;
>> - }
>> + ret = parse_sample_actions_attribute(nla, &sample);
>> } 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);
>> - if (!tc_action->sample.rate) {
>> - break;
>> - }
>> + rate = UINT32_MAX / nl_attr_get_u32(nla);
>> } else {
>> - break;
>> + return EINVAL;
>> }
>> }
>>
>> - if (!tc_action->sample.rate || err) {
>> + /* This check makes sure both attributes above were present and valid.
>> */
>> + if (!rate || ret) {
>> return EINVAL;
>> }
>>
>> @@ -2194,10 +2191,13 @@ parse_sample_action(struct tc_flower *flower, struct
>> tc_action *tc_action,
>> VLOG_DBG_RL(&rl, "Failed allocating group id for sample action");
>> return ENOENT;
>> }
>> +
>> + tc_action->type = TC_ACT_SAMPLE;
>> + tc_action->sample.rate = rate;
>> tc_action->sample.group_id = sgid;
>> flower->action_count++;
>>
>> - return err;
>> + return 0;
>> }
>>
>> static int
>> @@ -2232,7 +2232,7 @@ parse_userspace_action(struct tc_flower *flower,
>> struct tc_action *tc_action,
>> tc_action->sample.rate = 1;
>> flower->action_count++;
>>
>> - return err;
>> + return 0;
>> }
>>
>> static int
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
