On 6/20/24 09:35, Sunyang Wu via dev wrote:
> Add the "set dscp action" parsing function,
> so that the "set dscp action" can be offloaded.
>
> Signed-off-by: Sunyang Wu <[email protected]>
> ---
> lib/netdev-offload-dpdk.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
Hi, Sunyang Wu. Thanks for the patch!
See some comments inline.
Best regards, Ilya Maximets.
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 623005b1c..524942457 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -791,6 +791,17 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
> ds_put_format(s, "port %"PRIu16" ", ntohs(set_tp->port));
> }
> ds_put_cstr(s, "/ ");
> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP ||
> + actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP) {
> + const struct rte_flow_action_set_dscp *set_dscp = actions->conf;
> + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> + ? "set_ipv4_dscp " : "set_ipv6_dscp ";
> +
> + ds_put_cstr(s, dirstr);
> + if (set_dscp) {
> + ds_put_format(s, "dscp_value %d ", set_dscp->dscp);
> + }
> + ds_put_cstr(s, "/ ");
> } else if (actions->type == RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN) {
> const struct rte_flow_action_of_push_vlan *of_push_vlan =
> actions->conf;
> @@ -1835,7 +1846,10 @@ add_set_flow_action__(struct flow_actions *actions,
> if (is_all_zeros(mask, size)) {
> return 0;
> }
> - if (!is_all_ones(mask, size)) {
> + if (!is_all_ones(mask, size) &&
> + /* set dscp need patial mask */
> + attr != RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP &&
> + attr != RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP) {
This doesn't seem right. We need to check that the mask contains
all the DSCP bits and that it doesn't have anything else inside.
For example, if we try to set ECN bits, we'll have them in a mask.
In this case is_all_zeros() check will fail and then this check
will allow us to proceed as well. In the end, we'll end up setting
DSCP bits to all zeroes, or worse, to whatever was in the first
six bits of nw_tos.
> VLOG_DBG_RL(&rl, "Partial mask is not supported");
> return -1;
> }
> @@ -1912,6 +1926,7 @@ parse_set_actions(struct flow_actions *actions,
> add_set_flow_action(ipv4_src, RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC);
> add_set_flow_action(ipv4_dst, RTE_FLOW_ACTION_TYPE_SET_IPV4_DST);
> add_set_flow_action(ipv4_ttl, RTE_FLOW_ACTION_TYPE_SET_TTL);
> + add_set_flow_action(ipv4_tos,
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP);z
>
> if (mask && !is_all_zeros(mask, sizeof *mask)) {
> VLOG_DBG_RL(&rl, "Unsupported IPv4 set action");
> @@ -1924,6 +1939,8 @@ parse_set_actions(struct flow_actions *actions,
> add_set_flow_action(ipv6_src, RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC);
> add_set_flow_action(ipv6_dst, RTE_FLOW_ACTION_TYPE_SET_IPV6_DST);
> add_set_flow_action(ipv6_hlimit, RTE_FLOW_ACTION_TYPE_SET_TTL);
> + add_set_flow_action(ipv6_tclass,
> + RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP);
>
> if (mask && !is_all_zeros(mask, sizeof *mask)) {
> VLOG_DBG_RL(&rl, "Unsupported IPv6 set action");
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
