On Mon, Mar 28, 2022 at 7:11 AM Eelco Chaudron <echau...@redhat.com> wrote:
>
> This change implements support for the check_pkt_len
> action using the TC police action, which supports MTU
> checking.
>
> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
Hello Eelco, I've run into a few problems with this patch.
I've found that the following invocations will cause a core dump:
ovs-ofctl add-flow br0
"table=0,udp,actions=load:0x1->NXM_NX_REG1[],resubmit(,1)"
ovs-ofctl add-flow br0
"table=1,reg1=0x1,actions=check_pkt_larger(20)->NXM_NX_REG0[1],
check_pkt_larger(40)->NXM_NX_REG1[1],check_pkt_larger(60)->NXM_NX_REG2[1],
check_pkt_larger(80)->NXM_NX_REG3[1],check_pkt_larger(100)->NXM_NX_REG4[1],
check_pkt_larger(110)->NXM_NX_REG5[1],check_pkt_larger(120)->NXM_NX_REG6[1],
check_pkt_larger(140)->NXM_NX_REG7[1],check_pkt_larger(160)->NXM_NX_REG8[1],
check_pkt_larger(180)->NXM_NX_REG9[1],check_pkt_larger(200)->NXM_NX_REG10[1],
check_pkt_larger(220)->NXM_NX_REG11[1],resubmit(,2)"
Secondly, and I think this one is my mistake somehow, the following invocation
ovs-ofctl add-flow br0
"table=0,udp,actions=load:0x1->NXM_NX_REG1[],resubmit(,1)"
ovs-ofctl add-flow br0
"table=1,reg1=0x1,actions=check_pkt_larger(800)->NXM_NX_REG0[1],resubmit(,2)"
ovs-ofctl add-flow br0 "table=2,reg0=0x1,actions=drop"
ovs-ofctl add-flow br0 "table=2,reg0=0x0,actions=normal"
Will produce the following flow rule action in dpctl:
actions:check_pkt_len(size=800,gt(drop),le(drop))
For the first issue, I think there should be some protection against
over-recursion. And the second, I think I'm doing something wrong.
Cheers,
M
> ---
> lib/netdev-offload-tc.c | 171 +++++++++++++++++-
> lib/tc.c | 455
> +++++++++++++++++++++++++++++++++++++++++++----
> lib/tc.h | 12 +
> 3 files changed, 594 insertions(+), 44 deletions(-)
>
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index a7838e503..bc15244cf 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -62,6 +62,13 @@ struct chain_node {
> uint32_t chain;
> };
>
> +static int
> +netdev_tc_parse_nl_actions(struct netdev *netdev, struct tc_flower *flower,
> + struct offload_info *info,
> + const struct nlattr *actions, size_t actions_len,
> + bool *recirc_act, bool more_actions,
> + struct tc_action **need_jump_update);
> +
> static bool
> is_internal_port(const char *type)
> {
> @@ -825,6 +832,45 @@ _parse_tc_flower_to_actions(struct tc_flower *flower,
> struct ofpbuf *buf,
> nl_msg_put_u32(buf, OVS_ACTION_ATTR_RECIRC, action->chain);
> }
> break;
> + case TC_ACT_POLICE_MTU: {
> + size_t offset, act_offset;
> + uint32_t jump;
> +
> + offset = nl_msg_start_nested(buf,
> + OVS_ACTION_ATTR_CHECK_PKT_LEN);
> +
> + nl_msg_put_u16(buf, OVS_CHECK_PKT_LEN_ATTR_PKT_LEN,
> + action->police.mtu);
> +
> + act_offset = nl_msg_start_nested(
> + buf, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER);
> + i = _parse_tc_flower_to_actions(flower, buf, i + 1,
> + action->police.result_jump);
> + nl_msg_end_nested(buf, act_offset);
> +
> + act_offset = nl_msg_start_nested(
> + buf, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL);
> +
> + jump = flower->actions[i - 1].jump_action;
> + if (jump == JUMP_ACTION_STOP) {
> + jump = max_index;
> + }
> + if (jump != 0) {
> + i = _parse_tc_flower_to_actions(flower, buf, i, jump);
> + }
> + nl_msg_end_nested(buf, act_offset);
> +
> + i--;
> + nl_msg_end_nested(buf, offset);
> + }
> + break;
> + }
> +
> + if (action->jump_action && action->type != TC_ACT_POLICE_MTU) {
> + /* If there is a jump, it means this was the end of an action
> + * set and we need to end this branch. */
> + i++;
> + break;
> }
> }
> return i;
> @@ -1578,11 +1624,121 @@ parse_match_ct_state_to_flower(struct tc_flower
> *flower, struct match *match)
> }
> }
>
> +
> +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 tc_action **need_jump_update,
> + bool *recirc_act)
> +{
> + struct tc_action *ge_jump_update = NULL, *le_jump_update = NULL;
> + const struct nlattr *nl_actions;
> + int err, le_offset, gt_offset;
> + uint16_t pkt_len;
> +
> + static const struct nl_policy ovs_cpl_policy[] = {
> + [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = { .type = NL_A_U16 },
> + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = { .type = NL_A_NESTED
> },
> + [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL]
> + = { .type = NL_A_NESTED },
> + };
> + struct nlattr *a[ARRAY_SIZE(ovs_cpl_policy)];
> +
> + if (!nl_parse_nested(nla, ovs_cpl_policy, a, ARRAY_SIZE(a))) {
> + VLOG_INFO("Received invalid formatted
> OVS_ACTION_ATTR_CHECK_PKT_LEN!");
> + return EOPNOTSUPP;
> + }
> +
> + if (!a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] ||
> + !a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL]) {
> + VLOG_INFO("Received invalid OVS_CHECK_PKT_LEN_ATTR_ACTION_IF_*!");
> + return EOPNOTSUPP;
> + }
> +
> + pkt_len = nl_attr_get_u16(a[OVS_CHECK_PKT_LEN_ATTR_PKT_LEN]);
> +
> + /* Add the police mtu action first in the allocated slot. */
> + action->police.mtu = pkt_len;
> + action->type = TC_ACT_POLICE_MTU;
> + le_offset = flower->action_count++;
> +
> + /* Parse and add the greater than action(s).
> + * 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,
> + nl_attr_get(nl_actions),
> + nl_attr_get_size(nl_actions),
> + recirc_act, !last_action,
> + &ge_jump_update);
> + if (err) {
> + return err;
> + }
> +
> + /* Update goto offset for le actions */
> + flower->actions[le_offset].police.result_jump = flower->action_count;
> +
> + gt_offset = flower->action_count;
> +
> + /* 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,
> + nl_attr_get(nl_actions),
> + nl_attr_get_size(nl_actions),
> + recirc_act, !last_action,
> + &le_jump_update);
> +
> + if (gt_offset == flower->action_count && last_action) {
> + /* No le actions where added, fix gt offset */
> + flower->actions[le_offset].police.result_jump = JUMP_ACTION_STOP;
> + }
> +
> + /* Update goto offset for gt actions to skip the le ones. */
> + if (last_action) {
> + flower->actions[gt_offset - 1].jump_action = JUMP_ACTION_STOP;
> +
> + if (need_jump_update) {
> + *need_jump_update = NULL;
> + }
> + } else {
> + if (gt_offset == flower->action_count) {
> + flower->actions[gt_offset - 1].jump_action = 0;
> + } else {
> + flower->actions[gt_offset - 1].jump_action =
> flower->action_count;
> + }
> + /* If we have nested if() else () the if actions jump over the else
> + * and will end-up in the outer else () case, which it should have
> + * skipped. To void this we return the "potential" inner if() goto to
> + * need_jump_update, so it can be updated on return!
> + */
> + if (need_jump_update) {
> + *need_jump_update = &flower->actions[gt_offset - 1];
> + }
> + }
> +
> + if (le_jump_update != NULL) {
> + le_jump_update->jump_action =
> + flower->actions[gt_offset - 1].jump_action;
> + }
> + if (ge_jump_update != NULL) {
> + ge_jump_update->jump_action =
> + flower->actions[gt_offset - 1].jump_action;
> + }
> +
> + if (err) {
> + return err;
> + }
> +
> + return 0;
> +}
> +
> static int
> netdev_tc_parse_nl_actions(struct netdev *netdev, struct tc_flower *flower,
> struct offload_info *info,
> const struct nlattr *actions, size_t actions_len,
> - bool *recirc_act)
> + bool *recirc_act, bool more_actions,
> + struct tc_action **need_jump_update)
> {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> const struct nlattr *nla;
> @@ -1702,6 +1858,16 @@ netdev_tc_parse_nl_actions(struct netdev *netdev,
> struct tc_flower *flower,
> action->type = TC_ACT_GOTO;
> action->chain = 0; /* 0 is reserved and not used by recirc. */
> 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,
> + nl_attr_len_pad(nla,
> + left) >= left
> + && !more_actions,
> + need_jump_update,
> + recirc_act);
> + if (err) {
> + return err;
> + }
> } else {
> VLOG_DBG_RL(&rl, "unsupported put action type: %d",
> nl_attr_type(nla));
> @@ -1974,7 +2140,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
> *match,
>
> /* Parse all (nested) actions. */
> err = netdev_tc_parse_nl_actions(netdev, &flower, info,
> - actions, actions_len, &recirc_act);
> + actions, actions_len, &recirc_act,
> + false, NULL);
> if (err) {
> return err;
> }
> diff --git a/lib/tc.c b/lib/tc.c
> index df73a43d4..812b0432c 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -994,6 +994,18 @@ nl_parse_flower_flags(struct nlattr **attrs, struct
> tc_flower *flower)
> flower->offloaded_state = nl_get_flower_offloaded_state(attrs);
> }
>
> +static void
> +nl_parse_action_pc(uint32_t action_pc, struct tc_action *action)
> +{
> + if (action_pc == TC_ACT_STOLEN) {
> + action->jump_action = JUMP_ACTION_STOP;
> + } else if (action_pc & TC_ACT_JUMP) {
> + action->jump_action = action_pc & TC_ACT_EXT_VAL_MASK;
> + } else {
> + action->jump_action = 0;
> + }
> +}
> +
> static const struct nl_policy pedit_policy[] = {
> [TCA_PEDIT_PARMS_EX] = { .type = NL_A_UNSPEC,
> .min_len = sizeof(struct tc_pedit),
> @@ -1093,6 +1105,7 @@ nl_parse_act_pedit(struct nlattr *options, struct
> tc_flower *flower)
>
> action->type = TC_ACT_PEDIT;
>
> + nl_parse_action_pc(pe->action, action);
> return 0;
> }
>
> @@ -1267,6 +1280,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct
> tc_flower *flower)
> if (err) {
> return err;
> }
> + nl_parse_action_pc(tun->action, action);
> } else if (tun->t_action == TCA_TUNNEL_KEY_ACT_RELEASE) {
> flower->tunnel = true;
> } else {
> @@ -1303,7 +1317,11 @@ get_user_hz(void)
> static void
> nl_parse_tcf(const struct tcf_t *tm, struct tc_flower *flower)
> {
> - flower->lastused = time_msec() - (tm->lastuse * 1000 / get_user_hz());
> + uint64_t lastused = time_msec() - (tm->lastuse * 1000 / get_user_hz());
> +
> + if (flower->lastused < lastused) {
> + flower->lastused = lastused;
> + }
> }
>
> static int
> @@ -1328,6 +1346,7 @@ nl_parse_act_gact(struct nlattr *options, struct
> tc_flower *flower)
> action = &flower->actions[flower->action_count++];
> action->chain = p->action & TC_ACT_EXT_VAL_MASK;
> action->type = TC_ACT_GOTO;
> + nl_parse_action_pc(p->action, action);
> } else if (p->action != TC_ACT_SHOT) {
> VLOG_ERR_RL(&error_rl, "unknown gact action: %d", p->action);
> return EINVAL;
> @@ -1386,8 +1405,9 @@ nl_parse_act_mirred(struct nlattr *options, struct
> tc_flower *flower)
>
> mirred_tm = mirred_attrs[TCA_MIRRED_TM];
> tm = nl_attr_get_unspec(mirred_tm, sizeof *tm);
> - nl_parse_tcf(tm, flower);
>
> + nl_parse_tcf(tm, flower);
> + nl_parse_action_pc(m->action, action);
> return 0;
> }
>
> @@ -1516,6 +1536,7 @@ nl_parse_act_ct(struct nlattr *options, struct
> tc_flower *flower)
> }
> action->type = TC_ACT_CT;
>
> + nl_parse_action_pc(ct->action, action);
> return 0;
> }
>
> @@ -1561,6 +1582,8 @@ nl_parse_act_vlan(struct nlattr *options, struct
> tc_flower *flower)
> v->action, v->v_action);
> return EINVAL;
> }
> +
> + nl_parse_action_pc(v->action, action);
> return 0;
> }
>
> @@ -1654,6 +1677,7 @@ nl_parse_act_mpls(struct nlattr *options, struct
> tc_flower *flower)
> return EINVAL;
> }
>
> + nl_parse_action_pc(m->action, action);
> return 0;
> }
>
> @@ -1694,9 +1718,60 @@ nl_parse_act_csum(struct nlattr *options, struct
> tc_flower *flower)
> return EINVAL;
> }
>
> + /* The action_pc should be set on the previous action. */
> + if (flower->action_count < TCA_ACT_MAX_NUM) {
> + struct tc_action *action = &flower->actions[flower->action_count];
> +
> + nl_parse_action_pc(c->action, action);
> + }
> return 0;
> }
>
> +static const struct nl_policy police_policy[] = {
> + [TCA_POLICE_TBF] = { .type = NL_A_UNSPEC,
> + .min_len = sizeof(struct tc_police),
> + .optional = false, },
> + [TCA_POLICE_RESULT] = { .type = NL_A_U32, .optional = false, },
> + [TCA_POLICE_TM] = { .type = NL_A_UNSPEC,
> + .min_len = sizeof(struct tcf_t),
> + .optional = false, },
> +};
> +
> +static int
> +nl_parse_act_police_mtu(struct nlattr *options, struct tc_flower *flower)
> +{
> + struct nlattr *police_attrs[ARRAY_SIZE(police_policy)];
> + const struct tc_police *police;
> + struct nlattr *police_result;
> + struct tc_action *action;
> + const struct tcf_t *tm;
> + uint32_t action_pc;
> +
> + if (!nl_parse_nested(options, police_policy, police_attrs,
> + ARRAY_SIZE(police_policy))) {
> + VLOG_ERR_RL(&error_rl, "Failed to parse police action options");
> + return EPROTO;
> + }
> + police = nl_attr_get_unspec(police_attrs[TCA_CSUM_PARMS], sizeof
> *police);
> + police_result = police_attrs[TCA_POLICE_RESULT];
> +
> + action = &flower->actions[flower->action_count++];
> + action->type = TC_ACT_POLICE_MTU;
> + action_pc = nl_attr_get_u32(police_result);
> + if (action_pc & TC_ACT_JUMP) {
> + action->police.result_jump = action_pc & TC_ACT_EXT_VAL_MASK;
> + } else {
> + action->police.result_jump = JUMP_ACTION_STOP;
> + }
> + action->police.mtu = police->mtu;
> +
> + tm = nl_attr_get_unspec(police_attrs[TCA_POLICE_TM], sizeof *tm);
> + nl_parse_tcf(tm, flower);
> + nl_parse_action_pc(police->action, action);
> + return 0;
> +}
> +
> +
> static const struct nl_policy act_policy[] = {
> [TCA_ACT_KIND] = { .type = NL_A_STRING, .optional = false, },
> [TCA_ACT_COOKIE] = { .type = NL_A_UNSPEC, .optional = true, },
> @@ -1715,7 +1790,7 @@ static const struct nl_policy stats_policy[] = {
>
> static int
> nl_parse_single_action(struct nlattr *action, struct tc_flower *flower,
> - bool terse)
> + bool terse, bool *csum)
> {
> struct nlattr *act_options;
> struct nlattr *act_stats;
> @@ -1737,6 +1812,7 @@ nl_parse_single_action(struct nlattr *action, struct
> tc_flower *flower,
> return EPROTO;
> }
>
> + *csum = false;
> act_kind = nl_attr_get_string(action_attrs[TCA_ACT_KIND]);
> act_options = action_attrs[TCA_ACT_OPTIONS];
> act_cookie = action_attrs[TCA_ACT_COOKIE];
> @@ -1757,10 +1833,13 @@ nl_parse_single_action(struct nlattr *action, struct
> tc_flower *flower,
> err = nl_parse_act_pedit(act_options, flower);
> } else if (!strcmp(act_kind, "csum")) {
> nl_parse_act_csum(act_options, flower);
> + *csum = true;
> } else if (!strcmp(act_kind, "skbedit")) {
> /* 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, "police")) {
> + nl_parse_act_police_mtu(act_options, flower);
> } else {
> VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind);
> err = EINVAL;
> @@ -1818,6 +1897,10 @@ nl_parse_flower_actions(struct nlattr **attrs, struct
> tc_flower *flower,
> static struct nl_policy actions_orders_policy[TCA_ACT_MAX_NUM + 1] = {};
> struct nlattr *actions_orders[ARRAY_SIZE(actions_orders_policy)];
> const int max_size = ARRAY_SIZE(actions_orders_policy);
> + int previous_action_count = 0;
> + bool need_jump_adjust = false;
> + int jump_adjust = 0;
> + bool csum = false;
>
> for (int i = TCA_ACT_MIN_PRIO; i < max_size; i++) {
> actions_orders_policy[i].type = NL_A_NESTED;
> @@ -1838,7 +1921,70 @@ nl_parse_flower_actions(struct nlattr **attrs, struct
> tc_flower *flower,
> VLOG_DBG_RL(&error_rl, "Can only support %d actions",
> TCA_ACT_MAX_NUM);
> return EOPNOTSUPP;
> }
> - err = nl_parse_single_action(actions_orders[i], flower, terse);
> + err = nl_parse_single_action(actions_orders[i], flower, terse,
> + &csum);
> +
> + if (flower->action_count == previous_action_count) {
> +
> + struct tc_action *action;
> +
> + /* We had no update on the TC action count, which means
> + * we had a none TC type action. So need to adjust existing
> + * jump offsets. */
> + jump_adjust++;
> +
> + if (need_jump_adjust || (csum && flower->action_count > 0)) {
> +
> + if (csum && flower->action_count > 0) {
> + /* The csum action is special as it might carry
> + * a jump count for the previous TC_ACT and therefore
> + * should be adjusted with jump_adjust as it got
> + * copied. */
> + action = &flower->actions[flower->action_count - 1];
> + if (action->jump_action
> + && action->jump_action != JUMP_ACTION_STOP) {
> + action->jump_action -= (jump_adjust - 1);
> + }
> + }
> +
> + for (int j = 0; j < flower->action_count; j++) {
> + action = &flower->actions[j];
> +
> + if (action->type == TC_ACT_POLICE_MTU
> + && action->police.result_jump != JUMP_ACTION_STOP
> + && (action->police.result_jump - 1) >
> + flower->action_count) {
> +
> + action->police.result_jump--;
> + }
> +
> + if (action->jump_action
> + && action->jump_action != JUMP_ACTION_STOP
> + && (action->jump_action - 1) >
> + flower->action_count) {
> +
> + action->jump_action--;
> + }
> + }
> + }
> + } else {
> + struct tc_action *action;
> +
> + action = &flower->actions[previous_action_count];
> + if (action->type == TC_ACT_POLICE_MTU &&
> + action->police.result_jump != JUMP_ACTION_STOP) {
> + action->police.result_jump -= jump_adjust;
> + need_jump_adjust = true;
> + }
> +
> + if (action->jump_action
> + && action->jump_action != JUMP_ACTION_STOP) {
> + action->jump_action -= jump_adjust;
> + need_jump_adjust = true;
> + }
> +
> + previous_action_count = flower->action_count;
> + }
>
> if (err) {
> return err;
> @@ -2031,14 +2177,14 @@ tc_get_tc_cls_policy(enum tc_offload_policy policy)
> }
>
> static void
> -nl_msg_put_act_csum(struct ofpbuf *request, uint32_t flags)
> +nl_msg_put_act_csum(struct ofpbuf *request, uint32_t flags, uint32_t
> action_pc)
> {
> size_t offset;
>
> nl_msg_put_string(request, TCA_ACT_KIND, "csum");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
> {
> - struct tc_csum parm = { .action = TC_ACT_PIPE,
> + struct tc_csum parm = { .action = action_pc,
> .update_flags = flags };
>
> nl_msg_put_unspec(request, TCA_CSUM_PARMS, &parm, sizeof parm);
> @@ -2048,7 +2194,7 @@ nl_msg_put_act_csum(struct ofpbuf *request, uint32_t
> flags)
>
> static void
> nl_msg_put_act_pedit(struct ofpbuf *request, struct tc_pedit *parm,
> - struct tc_pedit_key_ex *ex)
> + struct tc_pedit_key_ex *ex, uint32_t action_pc)
> {
> size_t ksize = sizeof *parm + parm->nkeys * sizeof(struct tc_pedit_key);
> size_t offset, offset_keys_ex, offset_key;
> @@ -2057,7 +2203,7 @@ nl_msg_put_act_pedit(struct ofpbuf *request, struct
> tc_pedit *parm,
> nl_msg_put_string(request, TCA_ACT_KIND, "pedit");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
> {
> - parm->action = TC_ACT_PIPE;
> + parm->action = action_pc;
>
> nl_msg_put_unspec(request, TCA_PEDIT_PARMS_EX, parm, ksize);
> offset_keys_ex = nl_msg_start_nested(request, TCA_PEDIT_KEYS_EX);
> @@ -2074,14 +2220,14 @@ nl_msg_put_act_pedit(struct ofpbuf *request, struct
> tc_pedit *parm,
>
> static void
> nl_msg_put_act_push_vlan(struct ofpbuf *request, ovs_be16 tpid,
> - uint16_t vid, uint8_t prio)
> + uint16_t vid, uint8_t prio, uint32_t action_pc)
> {
> size_t offset;
>
> nl_msg_put_string(request, TCA_ACT_KIND, "vlan");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
> {
> - struct tc_vlan parm = { .action = TC_ACT_PIPE,
> + struct tc_vlan parm = { .action = action_pc,
> .v_action = TCA_VLAN_ACT_PUSH };
>
> nl_msg_put_unspec(request, TCA_VLAN_PARMS, &parm, sizeof parm);
> @@ -2093,14 +2239,14 @@ nl_msg_put_act_push_vlan(struct ofpbuf *request,
> ovs_be16 tpid,
> }
>
> static void
> -nl_msg_put_act_pop_vlan(struct ofpbuf *request)
> +nl_msg_put_act_pop_vlan(struct ofpbuf *request, uint32_t action_pc)
> {
> size_t offset;
>
> nl_msg_put_string(request, TCA_ACT_KIND, "vlan");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
> {
> - struct tc_vlan parm = { .action = TC_ACT_PIPE,
> + struct tc_vlan parm = { .action = action_pc,
> .v_action = TCA_VLAN_ACT_POP };
>
> nl_msg_put_unspec(request, TCA_VLAN_PARMS, &parm, sizeof parm);
> @@ -2109,14 +2255,15 @@ nl_msg_put_act_pop_vlan(struct ofpbuf *request)
> }
>
> static void
> -nl_msg_put_act_pop_mpls(struct ofpbuf *request, ovs_be16 proto)
> +nl_msg_put_act_pop_mpls(struct ofpbuf *request, ovs_be16 proto,
> + uint32_t action_pc)
> {
> size_t offset;
>
> nl_msg_put_string(request, TCA_ACT_KIND, "mpls");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS | NLA_F_NESTED);
> {
> - struct tc_mpls parm = { .action = TC_ACT_PIPE,
> + struct tc_mpls parm = { .action = action_pc,
> .m_action = TCA_MPLS_ACT_POP };
>
> nl_msg_put_unspec(request, TCA_MPLS_PARMS, &parm, sizeof parm);
> @@ -2127,14 +2274,15 @@ nl_msg_put_act_pop_mpls(struct ofpbuf *request,
> ovs_be16 proto)
>
> static void
> nl_msg_put_act_push_mpls(struct ofpbuf *request, ovs_be16 proto,
> - uint32_t label, uint8_t tc, uint8_t ttl, uint8_t
> bos)
> + uint32_t label, uint8_t tc, uint8_t ttl, uint8_t
> bos,
> + uint32_t action_pc)
> {
> size_t offset;
>
> nl_msg_put_string(request, TCA_ACT_KIND, "mpls");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS | NLA_F_NESTED);
> {
> - struct tc_mpls parm = { .action = TC_ACT_PIPE,
> + struct tc_mpls parm = { .action = action_pc,
> .m_action = TCA_MPLS_ACT_PUSH };
>
> nl_msg_put_unspec(request, TCA_MPLS_PARMS, &parm, sizeof parm);
> @@ -2149,14 +2297,14 @@ nl_msg_put_act_push_mpls(struct ofpbuf *request,
> ovs_be16 proto,
>
> static void
> nl_msg_put_act_set_mpls(struct ofpbuf *request, uint32_t label, uint8_t tc,
> - uint8_t ttl, uint8_t bos)
> + uint8_t ttl, uint8_t bos, uint32_t action_pc)
> {
> size_t offset;
>
> nl_msg_put_string(request, TCA_ACT_KIND, "mpls");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS | NLA_F_NESTED);
> {
> - struct tc_mpls parm = { .action = TC_ACT_PIPE,
> + struct tc_mpls parm = { .action = action_pc,
> .m_action = TCA_MPLS_ACT_MODIFY };
>
> nl_msg_put_unspec(request, TCA_MPLS_PARMS, &parm, sizeof parm);
> @@ -2225,14 +2373,14 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request,
> bool id_present,
> struct in6_addr *ipv6_dst,
> ovs_be16 tp_dst, uint8_t tos, uint8_t ttl,
> struct tun_metadata tun_metadata,
> - uint8_t no_csum)
> + uint8_t no_csum, uint32_t action_pc)
> {
> size_t offset;
>
> nl_msg_put_string(request, TCA_ACT_KIND, "tunnel_key");
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
> {
> - struct tc_tunnel_key tun = { .action = TC_ACT_PIPE,
> + struct tc_tunnel_key tun = { .action = action_pc,
> .t_action = TCA_TUNNEL_KEY_ACT_SET };
>
> nl_msg_put_unspec(request, TCA_TUNNEL_KEY_PARMS, &tun, sizeof tun);
> @@ -2285,7 +2433,8 @@ nl_msg_put_act_gact(struct ofpbuf *request, uint32_t
> chain)
> }
>
> static void
> -nl_msg_put_act_ct(struct ofpbuf *request, struct tc_action *action)
> +nl_msg_put_act_ct(struct ofpbuf *request, struct tc_action *action,
> + uint32_t action_pc)
> {
> uint16_t ct_action = 0;
> size_t offset;
> @@ -2294,7 +2443,7 @@ nl_msg_put_act_ct(struct ofpbuf *request, struct
> tc_action *action)
> offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS | NLA_F_NESTED);
> {
> struct tc_ct ct = {
> - .action = TC_ACT_PIPE,
> + .action = action_pc,
> };
>
> if (!action->ct.clear) {
> @@ -2499,10 +2648,57 @@ csum_update_flag(struct tc_flower *flower,
> return EOPNOTSUPP;
> }
>
> +static bool
> +rewrite_pedits_need_csum_update(struct tc_action *action)
> +{
> + int i, j;
> +
> + for (i = 0; i < ARRAY_SIZE(flower_pedit_map); i++) {
> + struct flower_key_to_pedit *m = &flower_pedit_map[i];
> + ovs_be32 *mask, *data, first_word_mask, last_word_mask;
> + int cnt = 0, cur_offset = 0;
> +
> + if (!m->size) {
> + continue;
> + }
> +
> + calc_offsets(action, m, &cur_offset, &cnt, &last_word_mask,
> + &first_word_mask, &mask, &data);
> +
> + for (j = 0; j < cnt; j++, mask++) {
> + ovs_be32 mask_word = *mask;
> +
> + if (j == 0) {
> + mask_word &= first_word_mask;
> + }
> + if (j == cnt - 1) {
> + mask_word &= last_word_mask;
> + }
> + if (!mask_word) {
> + continue;
> + }
> +
> + switch (m->htype) {
> + case TCA_PEDIT_KEY_EX_HDR_TYPE_IP4:
> + case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6:
> + case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP:
> + case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP:
> + return true;
> + case TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK:
> + case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH:
> + case __PEDIT_HDR_TYPE_MAX:
> + break;
> + }
> + }
> + }
> + return false;
> +}
> +
> static int
> nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
> struct tc_flower *flower,
> - struct tc_action *action)
> + struct tc_action *action,
> + uint32_t action_pc)
> {
> struct {
> struct tc_pedit sel;
> @@ -2569,7 +2765,8 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
> }
> }
> }
> - nl_msg_put_act_pedit(request, &sel.sel, sel.keys_ex);
> + nl_msg_put_act_pedit(request, &sel.sel, sel.keys_ex,
> + flower->csum_update_flags ? TC_ACT_PIPE :
> action_pc);
>
> return 0;
> }
> @@ -2590,7 +2787,7 @@ nl_msg_put_flower_acts_release(struct ofpbuf *request,
> uint16_t act_index)
> * last pedit action. */
> static void
> nl_msg_put_csum_act(struct ofpbuf *request, struct tc_flower *flower,
> - uint16_t *act_index)
> + uint32_t action_pc, uint16_t *act_index)
> {
> size_t act_offset;
>
> @@ -2600,7 +2797,7 @@ nl_msg_put_csum_act(struct ofpbuf *request, struct
> tc_flower *flower,
> }
>
> act_offset = nl_msg_start_nested(request, (*act_index)++);
> - nl_msg_put_act_csum(request, flower->csum_update_flags);
> + nl_msg_put_act_csum(request, flower->csum_update_flags, action_pc);
> nl_msg_put_act_flags(request);
> nl_msg_end_nested(request, act_offset);
>
> @@ -2608,6 +2805,124 @@ nl_msg_put_csum_act(struct ofpbuf *request, struct
> tc_flower *flower,
> flower->csum_update_flags = 0;
> }
>
> +static int
> +get_action_index_for_tc_actions(struct tc_flower *flower, uint16_t act_index,
> + struct tc_action *action, int action_count,
> + bool tunnel_key_released)
> +{
> + bool need_csum = false;
> +
> + if (action_count < 0) {
> + /* Only forward jumps are supported */
> + return -EINVAL;
> + }
> +
> + for (int i = 0; i < action_count; i++, action++) {
> + if (action->type != TC_ACT_PEDIT && need_csum) {
> + need_csum = false;
> + act_index++;
> + }
> +
> + switch (action->type) {
> +
> + case TC_ACT_OUTPUT:
> + if (!tunnel_key_released && flower->tunnel) {
> + act_index++;
> + tunnel_key_released = true;
> + }
> + if (action->out.ingress) {
> + act_index++;
> + }
> + act_index++;
> + break;
> +
> + case TC_ACT_ENCAP:
> + if (!tunnel_key_released && flower->tunnel) {
> + act_index++;
> + tunnel_key_released = true;
> + }
> + act_index++;
> + break;
> +
> + case TC_ACT_PEDIT:
> + if (!need_csum) {
> + need_csum = rewrite_pedits_need_csum_update(action);
> + }
> + if (i == (action_count - 1) && need_csum) {
> + need_csum = false;
> + act_index++;
> + }
> + act_index++;
> + break;
> +
> + case TC_ACT_POLICE_MTU:
> + case TC_ACT_VLAN_POP:
> + case TC_ACT_VLAN_PUSH:
> + case TC_ACT_MPLS_POP:
> + case TC_ACT_MPLS_PUSH:
> + case TC_ACT_MPLS_SET:
> + case TC_ACT_GOTO:
> + case TC_ACT_CT:
> + /* Increase act_index by one if we are sure this type of action
> + * will only add one tc action in the kernel. */
> + act_index++;
> + break;
> +
> + /* If we can't determine how many tc actions will be added by the
> + * kernel return -EOPNOTSUPP.
> + *
> + * Please do NOT add a default case. */
> + }
> + }
> +
> + return act_index;
> +}
> +
> +static int
> +nl_msg_put_act_police_mtu(struct ofpbuf *request, struct tc_flower *flower,
> + struct tc_action *action, uint32_t action_pc,
> + int action_index, uint16_t act_index, bool
> released)
> +{
> + uint32_t tc_action;
> + size_t offset;
> +
> + if (action->police.result_jump != JUMP_ACTION_STOP) {
> + int jump_index;
> + int action_count = action->police.result_jump - action_index - 1;
> +
> + jump_index = get_action_index_for_tc_actions(flower,
> + act_index - 1,
> + action + 1,
> + action_count,
> + released);
> + if (jump_index < 0) {
> + return -jump_index;
> + }
> + tc_action = TC_ACT_JUMP | (jump_index & TC_ACT_EXT_VAL_MASK);
> + } else {
> + tc_action = TC_ACT_STOLEN;
> + }
> +
> + nl_msg_put_string(request, TCA_ACT_KIND, "police");
> + offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
> + {
> + struct tc_police p = { .action = action_pc,
> + .mtu = action->police.mtu };
> +
> + nl_msg_put_unspec(request, TCA_POLICE_TBF, &p, sizeof p);
> +
> + /* The value in jump_action is the total number of TC_ACT_*
> + * we need to jump, not the actual number of TCA_ACT_KIND
> + * (act_index) actions. As certain TC_ACT_* actions can be
> + * translated into multiple TCA_ACT_KIND ones.
> + *
> + * See nl_msg_put_flower_acts() below for more details. */
> + nl_msg_put_u32(request, TCA_POLICE_RESULT, tc_action);
> + }
> + nl_msg_end_nested(request, offset);
> + return 0;
> +}
> +
> static int
> nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
> {
> @@ -2621,17 +2936,59 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> offset = nl_msg_start_nested(request, TCA_FLOWER_ACT);
> {
> int error;
> + uint32_t prev_action_pc = TC_ACT_PIPE;
>
> action = flower->actions;
> for (i = 0; i < flower->action_count; i++, action++) {
> - if (action->type != TC_ACT_PEDIT) {
> - nl_msg_put_csum_act(request, flower, &act_index);
> + uint32_t action_pc; /* Programmatic Control */
> +
> + if (!action->jump_action) {
> + action_pc = TC_ACT_PIPE;
> + } else if (action->jump_action == JUMP_ACTION_STOP) {
> + action_pc = TC_ACT_STOLEN;
> + } else {
> + /* The value in jump_action is the total number of TC_ACT_*
> + * we need to jump, not the actual number of TCA_ACT_KIND
> + * (act_index) actions. As certain TC_ACT_* actions can be
> + * translated into multiple TCA_ACT_KIND ones.
> + *
> + * If we can determine the number of actual actions being
> + * inserted we will update the count, if not we will return
> + * -EOPNOTSUPP.
> + */
> + int jump_index;
> + int act_index_start = act_index - 1;
> + int action_count = (action->jump_action &
> + TC_ACT_EXT_VAL_MASK) - i;
> +
> + if (flower->csum_update_flags &&
> + (action->type != TC_ACT_PEDIT
> + || prev_action_pc & TC_ACT_JUMP)) {
> + act_index_start++;
> + }
> +
> + jump_index = get_action_index_for_tc_actions(flower,
> + act_index_start,
> + action,
> + action_count,
> + released);
> + if (jump_index < 0) {
> + return -jump_index;
> + }
> +
> + action_pc = TC_ACT_JUMP | jump_index;
> }
> +
> + if (action->type != TC_ACT_PEDIT || prev_action_pc &
> TC_ACT_JUMP) {
> + nl_msg_put_csum_act(request, flower, prev_action_pc,
> + &act_index);
> + }
> +
> switch (action->type) {
> case TC_ACT_PEDIT: {
> act_offset = nl_msg_start_nested(request, act_index++);
> error = nl_msg_put_flower_rewrite_pedits(request, flower,
> - action);
> + action, action_pc);
> if (error) {
> return error;
> }
> @@ -2639,7 +2996,8 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
>
> if (i == flower->action_count - 1) {
> /* If this is the last action check csum calc again. */
> - nl_msg_put_csum_act(request, flower, &act_index);
> + nl_msg_put_csum_act(request, flower, action_pc,
> + &act_index);
> }
> }
> break;
> @@ -2660,14 +3018,15 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> action->encap.tos,
> action->encap.ttl,
> action->encap.data,
> - action->encap.no_csum);
> + action->encap.no_csum,
> + action_pc);
> nl_msg_put_act_flags(request);
> nl_msg_end_nested(request, act_offset);
> }
> break;
> case TC_ACT_VLAN_POP: {
> act_offset = nl_msg_start_nested(request, act_index++);
> - nl_msg_put_act_pop_vlan(request);
> + nl_msg_put_act_pop_vlan(request, action_pc);
> nl_msg_put_act_flags(request);
> nl_msg_end_nested(request, act_offset);
> }
> @@ -2677,14 +3036,16 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> nl_msg_put_act_push_vlan(request,
> action->vlan.vlan_push_tpid,
> action->vlan.vlan_push_id,
> - action->vlan.vlan_push_prio);
> + action->vlan.vlan_push_prio,
> + action_pc);
> nl_msg_put_act_flags(request);
> nl_msg_end_nested(request, act_offset);
> }
> break;
> case TC_ACT_MPLS_POP: {
> act_offset = nl_msg_start_nested(request, act_index++);
> - nl_msg_put_act_pop_mpls(request, action->mpls.proto);
> + nl_msg_put_act_pop_mpls(request, action->mpls.proto,
> + action_pc);
> nl_msg_end_nested(request, act_offset);
> }
> break;
> @@ -2692,7 +3053,8 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> act_offset = nl_msg_start_nested(request, act_index++);
> nl_msg_put_act_push_mpls(request, action->mpls.proto,
> action->mpls.label, action->mpls.tc,
> - action->mpls.ttl, action->mpls.bos);
> + action->mpls.ttl, action->mpls.bos,
> + action_pc);
> nl_msg_end_nested(request, act_offset);
> }
> break;
> @@ -2700,7 +3062,7 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> act_offset = nl_msg_start_nested(request, act_index++);
> nl_msg_put_act_set_mpls(request, action->mpls.label,
> action->mpls.tc, action->mpls.ttl,
> - action->mpls.bos);
> + action->mpls.bos, action_pc);
> nl_msg_end_nested(request, act_offset);
> }
> break;
> @@ -2735,12 +3097,13 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> nl_msg_put_act_mirred(request, ifindex,
> TC_ACT_STOLEN,
> TCA_EGRESS_REDIR);
> }
> + action->jump_action = JUMP_ACTION_STOP;
> } else {
> if (ingress) {
> - nl_msg_put_act_mirred(request, ifindex, TC_ACT_PIPE,
> + nl_msg_put_act_mirred(request, ifindex, action_pc,
> TCA_INGRESS_MIRROR);
> } else {
> - nl_msg_put_act_mirred(request, ifindex, TC_ACT_PIPE,
> + nl_msg_put_act_mirred(request, ifindex, action_pc,
> TCA_EGRESS_MIRROR);
> }
> }
> @@ -2768,12 +3131,26 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> break;
> case TC_ACT_CT: {
> act_offset = nl_msg_start_nested(request, act_index++);
> - nl_msg_put_act_ct(request, action);
> + nl_msg_put_act_ct(request, action, action_pc);
> nl_msg_put_act_cookie(request, &flower->act_cookie);
> nl_msg_end_nested(request, act_offset);
> }
> break;
> + case TC_ACT_POLICE_MTU: {
> + act_offset = nl_msg_start_nested(request, act_index++);
> + if (nl_msg_put_act_police_mtu(request, flower, action,
> + action_pc, i, act_index,
> + released)) {
> + return -EOPNOTSUPP;
> + }
> + nl_msg_put_act_cookie(request, &flower->act_cookie);
> + nl_msg_put_act_flags(request);
> + nl_msg_end_nested(request, act_offset);
> + }
> + break;
> }
> +
> + prev_action_pc = action_pc;
> }
> }
>
> diff --git a/lib/tc.h b/lib/tc.h
> index d6cdddd16..7ccd71d01 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_POLICE_MTU,
> };
>
> enum nat_type {
> @@ -256,14 +257,19 @@ struct tc_action {
> bool force;
> bool commit;
> } ct;
> -
> struct {
> struct tc_flower_key key;
> struct tc_flower_key mask;
> } rewrite;
> - };
> + struct {
> + uint32_t result_jump;
> + uint16_t mtu;
> + } police;
> + };
>
> - enum tc_action_type type;
> + enum tc_action_type type;
> + uint32_t jump_action;
> +#define JUMP_ACTION_STOP 0xffffffff
> };
>
> /* assert that if we overflow with a masked write of uint32_t to the last
> byte
>
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
