On 1/12/26 12:20 PM, Eelco Chaudron wrote:
> Remove the netdev-offload dependencies from netdev-offload-tc,
> and have it fully controlled by the dpif-netdev-tc layer.
>
> Acked-by: Eli Britstein <elibr.nvidia.com>
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
> lib/dpif-offload-tc.c | 26 ++++++++++-
> lib/netdev-offload-tc.c | 96 +++++++++++++++++++++++------------------
> lib/netdev-offload-tc.h | 17 +++++++-
> lib/netdev-offload.c | 10 -----
> lib/netdev-offload.h | 7 ---
> lib/netdev.c | 12 +++++-
> lib/netdev.h | 1 +
> 7 files changed, 104 insertions(+), 65 deletions(-)
>
> diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
> index faf9e7867..a14f12151 100644
> --- a/lib/dpif-offload-tc.c
> +++ b/lib/dpif-offload-tc.c
> @@ -78,7 +78,14 @@ static int
> dpif_offload_tc_enable_offload(struct dpif_offload *dpif_offload,
> struct dpif_offload_port_mgr_port *port)
> {
> + int ret = netdev_offload_tc_init(port->netdev);
> + if (ret) {
> + VLOG_WARN("%s: Failed assigning flow API 'tc', error %d",
> + netdev_get_name(port->netdev), ret);
> + return ret;
> + }
> dpif_offload_set_netdev_offload(port->netdev, dpif_offload);
> + VLOG_INFO("%s: Assigned flow API 'tc'", netdev_get_name(port->netdev));
> return 0;
> }
>
> @@ -602,7 +609,7 @@ dpif_offload_tc_parse_flow_put(struct dpif_offload_tc
> *offload_tc,
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> struct dpif_offload_port_mgr_port *port;
> const struct nlattr *nla;
> - struct offload_info info;
> + struct tc_offload_info info;
> struct match match;
> odp_port_t in_port;
> size_t left;
> @@ -647,7 +654,7 @@ dpif_offload_tc_parse_flow_put(struct dpif_offload_tc
> *offload_tc,
>
> info.recirc_id_shared_with_tc = offload_tc->recirc_id_shared;
>
> - err = netdev_offload_tc_flow_put(port->netdev, &match,
> + err = netdev_offload_tc_flow_put(dpif, port->netdev, &match,
> CONST_CAST(struct nlattr *,
> put->actions),
> put->actions_len,
> CONST_CAST(ovs_u128 *, put->ufid),
> @@ -829,6 +836,21 @@ dpif_offload_tc_operate(struct dpif *dpif, const struct
> dpif_offload *offload,
> }
> }
>
> +odp_port_t
> +dpif_offload_tc_get_port_id_by_ifindex(const struct dpif_offload *offload,
> + int ifindex)
> +{
> + struct dpif_offload_tc *offload_tc = dpif_offload_tc_cast(offload);
> + struct dpif_offload_port_mgr_port *port;
> +
> + port = dpif_offload_port_mgr_find_by_ifindex(offload_tc->port_mgr,
> + ifindex);
> + if (port) {
> + return port->port_no;
> + }
> + return ODPP_NONE;
> +}
> +
> struct dpif_offload_class dpif_offload_tc_class = {
> .type = "tc",
> .impl_type = DPIF_OFFLOAD_IMPL_FLOWS_PROVIDER_ONLY,
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index f4195a95e..6a1b58438 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -31,7 +31,6 @@
> #include "openvswitch/util.h"
> #include "openvswitch/vlog.h"
> #include "netdev-linux.h"
> -#include "netdev-offload-provider.h"
> #include "netdev-offload-tc.h"
> #include "netdev-provider.h"
> #include "netdev-vport.h"
> @@ -98,9 +97,9 @@ static struct hmap police_idx_to_meter_id
> OVS_GUARDED_BY(meter_mutex)
> static int meter_id_lookup(uint32_t meter_id, uint32_t *police_idx);
> static int police_idx_lookup(uint32_t police_idx, uint32_t *meter_id);
>
> -static int netdev_tc_parse_nl_actions(struct netdev *netdev,
> +static int netdev_tc_parse_nl_actions(struct dpif *, struct netdev *netdev,
> struct tc_flower *flower,
> - struct offload_info *info,
> + struct tc_offload_info *info,
> const struct nlattr *actions,
> size_t actions_len,
> bool *recirc_act, bool more_actions,
> @@ -829,8 +828,22 @@ parse_tc_flower_terse_to_match(struct tc_flower *flower,
> return 0;
> }
>
> +static odp_port_t
> +netdev_tc_get_port_id_by_ifindex(const struct netdev *in_netdev, int ifindex)
> +{
> + const struct dpif_offload *offload = ovsrcu_get(
> + const struct dpif_offload *, &in_netdev->dpif_offload);
> +
> + if (!offload || strcmp(dpif_offload_class_type(offload), "tc")) {
> + return ODPP_NONE;
> + }
> +
> + return dpif_offload_tc_get_port_id_by_ifindex(offload, ifindex);
> +}
> +
> static int
> -parse_tc_flower_to_actions__(struct tc_flower *flower, struct ofpbuf *buf,
> +parse_tc_flower_to_actions__(const struct netdev *netdev,
> + struct tc_flower *flower, struct ofpbuf *buf,
> int start_index, int max_index)
> {
> struct tc_action *action;
> @@ -953,12 +966,12 @@ parse_tc_flower_to_actions__(struct tc_flower *flower,
> struct ofpbuf *buf,
> }
> break;
> case TC_ACT_OUTPUT: {
> - odp_port_t outport = 0;
> + odp_port_t outport = ODPP_NONE;
>
> if (action->out.ifindex_out) {
> - outport =
> - netdev_ifindex_to_odp_port(action->out.ifindex_out);
> - if (!outport) {
> + outport = netdev_tc_get_port_id_by_ifindex(
> + netdev, action->out.ifindex_out);
nit: maybe shift this line ~10 spaces to the right.
> + if (outport == ODPP_NONE) {
> return -ENOENT;
> }
> }
> @@ -1069,7 +1082,7 @@ parse_tc_flower_to_actions__(struct tc_flower *flower,
> struct ofpbuf *buf,
>
> 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,
> + i = parse_tc_flower_to_actions__(netdev, flower, buf, i + 1,
> action->police.result_jump);
> if (i < 0) {
> return i;
> @@ -1084,7 +1097,7 @@ parse_tc_flower_to_actions__(struct tc_flower *flower,
> struct ofpbuf *buf,
> jump = max_index;
> }
> if (jump != 0) {
> - i = parse_tc_flower_to_actions__(flower, buf, i, jump);
> + i = parse_tc_flower_to_actions__(netdev, flower, buf, i,
> jump);
> if (i < 0) {
> return i;
> }
> @@ -1108,10 +1121,11 @@ parse_tc_flower_to_actions__(struct tc_flower
> *flower, struct ofpbuf *buf,
> }
>
> static int
> -parse_tc_flower_to_actions(struct tc_flower *flower,
> +parse_tc_flower_to_actions(const struct netdev *netdev,
> + struct tc_flower *flower,
> struct ofpbuf *buf)
> {
> - return parse_tc_flower_to_actions__(flower, buf, 0, 0);
> + return parse_tc_flower_to_actions__(netdev, flower, buf, 0, 0);
> }
>
> static int
> @@ -1336,7 +1350,7 @@ parse_tc_flower_to_match(const struct netdev *netdev,
> }
>
> act_off = nl_msg_start_nested(buf, OVS_FLOW_ATTR_ACTIONS);
> - err = parse_tc_flower_to_actions(flower, buf);
> + err = parse_tc_flower_to_actions(netdev, flower, buf);
> if (err < 0) {
> return -err;
> }
> @@ -2041,11 +2055,12 @@ 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,
> +parse_check_pkt_len_action(struct dpif *dpif, struct netdev *netdev,
> + struct tc_flower *flower,
> + struct tc_offload_info *info,
> + struct tc_action *action, const struct nlattr
> *nla,
> + bool last_action,
> struct tc_action **need_jump_update,
> bool *recirc_act)
> {
> @@ -2084,7 +2099,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(dpif, netdev, flower, info,
> nl_attr_get(nl_actions),
> nl_attr_get_size(nl_actions),
> recirc_act, !last_action,
> @@ -2100,7 +2115,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(dpif, netdev, flower, info,
> nl_attr_get(nl_actions),
> nl_attr_get_size(nl_actions),
> recirc_act, !last_action,
> @@ -2151,8 +2166,9 @@ 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,
> +netdev_tc_parse_nl_actions(struct dpif *dpif, struct netdev *netdev,
> + struct tc_flower *flower,
> + struct tc_offload_info *info,
> const struct nlattr *actions, size_t actions_len,
> bool *recirc_act, bool more_actions,
> struct tc_action **need_jump_update)
> @@ -2174,12 +2190,14 @@ netdev_tc_parse_nl_actions(struct netdev *netdev,
> struct tc_flower *flower,
>
> if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
> odp_port_t port = nl_attr_get_odp_port(nla);
> - struct netdev *outdev = netdev_ports_get(
> - port, netdev_get_dpif_type(netdev));
> -
> + struct netdev *outdev = dpif_offload_get_netdev_by_port_id(dpif,
> + NULL,
> + port);
nit: Split after the '='.
> if (!outdev) {
> - VLOG_DBG_RL(&rl, "Can't find netdev for output port %d",
> port);
> - return ENODEV;
> + VLOG_DBG_RL(&rl,
> + "Can't find offloaded netdev for output port %d",
> + port);
nit: Maybe:
VLOG_DBG_RL(&rl,
"Can't find offloaded netdev for output port %d", port);
?
> + return EOPNOTSUPP;
> }
>
> if (!dpif_offload_netdev_same_offload(netdev, outdev)) {
> @@ -2187,7 +2205,6 @@ netdev_tc_parse_nl_actions(struct netdev *netdev,
> struct tc_flower *flower,
> "Flow API provider mismatch between ingress (%s)
> "
> "and egress (%s) ports",
> netdev_get_name(netdev),
> netdev_get_name(outdev));
> - netdev_close(outdev);
> return EOPNOTSUPP;
> }
>
> @@ -2196,14 +2213,12 @@ netdev_tc_parse_nl_actions(struct netdev *netdev,
> struct tc_flower *flower,
> VLOG_DBG_RL(&rl,
> "Can't find ifindex for output port %s, error
> %d",
> netdev_get_name(outdev),
> action->out.ifindex_out);
> - netdev_close(outdev);
> return -action->out.ifindex_out;
> }
>
> action->out.ingress = is_internal_port(netdev_get_type(outdev));
> action->type = TC_ACT_OUTPUT;
> flower->action_count++;
> - netdev_close(outdev);
> } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_PUSH_VLAN) {
> const struct ovs_action_push_vlan *vlan_push = nl_attr_get(nla);
>
> @@ -2282,7 +2297,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(dpif, netdev, flower, info,
> + action, nla,
> nl_attr_len_pad(nla,
> left) >= left
> && !more_actions,
> @@ -2301,9 +2317,10 @@ netdev_tc_parse_nl_actions(struct netdev *netdev,
> struct tc_flower *flower,
> }
>
> int
> -netdev_offload_tc_flow_put(struct netdev *netdev, struct match *match,
> - struct nlattr *actions, size_t actions_len,
> - const ovs_u128 *ufid, struct offload_info *info,
> +netdev_offload_tc_flow_put(struct dpif *dpif, struct netdev *netdev,
> + struct match *match, struct nlattr *actions,
> + size_t actions_len, const ovs_u128 *ufid,
> + struct tc_offload_info *info,
> struct dpif_flow_stats *stats)
> {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> @@ -2656,7 +2673,7 @@ netdev_offload_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(dpif, netdev, &flower, info,
> actions, actions_len, &recirc_act,
> false, NULL);
> if (err) {
> @@ -2753,7 +2770,7 @@ netdev_offload_tc_flow_get(struct netdev *netdev,
> return err;
> }
>
> - in_port = netdev_ifindex_to_odp_port(id.ifindex);
> + in_port = netdev_tc_get_port_id_by_ifindex(netdev, id.ifindex);
> err = parse_tc_flower_to_match(netdev, &flower, match, actions,
> stats, attrs, buf, false);
> if (err) {
> @@ -3159,8 +3176,8 @@ dpif_offload_tc_meter_init(void)
> }
> }
>
> -static int
> -netdev_tc_init_flow_api(struct netdev *netdev)
> +int
> +netdev_offload_tc_init(struct netdev *netdev)
> {
> static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
> @@ -3438,8 +3455,3 @@ dpif_offload_tc_meter_del(const struct dpif_offload
> *offload OVS_UNUSED,
>
> return err;
> }
> -
> -const struct netdev_flow_api netdev_offload_tc = {
> - .type = "linux_tc",
> - .init_flow_api = netdev_tc_init_flow_api,
> -};
> diff --git a/lib/netdev-offload-tc.h b/lib/netdev-offload-tc.h
> index 5bab36176..e49a2d343 100644
> --- a/lib/netdev-offload-tc.h
> +++ b/lib/netdev-offload-tc.h
> @@ -29,8 +29,19 @@ struct netdev_tc_flow_dump {
> bool terse;
> };
>
> +/* Flow offloading. */
> +struct tc_offload_info {
> + bool recirc_id_shared_with_tc; /* Indicates whether tc chains will be in
> + * sync with datapath recirc ids. */
> +
> + bool tc_modify_flow; /* Indicates tc modified the flow. */
> + bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success
> + * to delete the original flow. */
> +};
> +
> /* Netdev-specific offload functions. These should only be used by the
> * associated dpif offload provider. */
> +int netdev_offload_tc_init(struct netdev *);
> int netdev_offload_tc_flow_flush(struct netdev *);
> int netdev_offload_tc_flow_dump_create(struct netdev *,
> struct netdev_tc_flow_dump **,
> @@ -42,9 +53,9 @@ bool netdev_offload_tc_flow_dump_next(struct
> netdev_tc_flow_dump *,
> struct dpif_flow_attrs *, ovs_u128
> *ufid,
> struct ofpbuf *rbuffer,
> struct ofpbuf *wbuffer);
> -int netdev_offload_tc_flow_put(struct netdev *, struct match *,
> +int netdev_offload_tc_flow_put(struct dpif *, struct netdev *, struct match
> *,
> struct nlattr *actions, size_t actions_len,
> - const ovs_u128 *ufid, struct offload_info *,
> + const ovs_u128 *ufid, struct tc_offload_info
> *,
> struct dpif_flow_stats *);
> int netdev_offload_tc_flow_del(const ovs_u128 *ufid, struct dpif_flow_stats
> *);
> int netdev_offload_tc_flow_get(struct netdev *, struct match *,
> @@ -59,5 +70,7 @@ int dpif_offload_tc_meter_get(const struct dpif_offload *,
> ofproto_meter_id,
> int dpif_offload_tc_meter_del(const struct dpif_offload *, ofproto_meter_id,
> struct ofputil_meter_stats *);
> uint64_t dpif_offload_tc_flow_get_n_offloaded(const struct dpif_offload *);
> +odp_port_t dpif_offload_tc_get_port_id_by_ifindex(const struct dpif_offload
> *,
> + int ifindex);
>
> #endif /* NETDEV_OFFLOAD_TC_H */
> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index 0db94c8f1..8d2e4fb74 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
> @@ -258,16 +258,6 @@ netdev_uninit_flow_api(struct netdev *netdev)
> ovs_refcount_unref(&rfa->refcnt);
> }
>
> -uint32_t
> -netdev_get_block_id(struct netdev *netdev)
> -{
> - const struct netdev_class *class = netdev->netdev_class;
> -
> - return (class->get_block_id
> - ? class->get_block_id(netdev)
> - : 0);
> -}
> -
> /*
> * Get the value of the hw info parameter specified by type.
> * Returns the value on success (>= 0). Returns -1 on failure.
> diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
> index 3bb577f5b..7877c40d9 100644
> --- a/lib/netdev-offload.h
> +++ b/lib/netdev-offload.h
> @@ -62,18 +62,12 @@ enum hw_info_type {
>
> /* Flow offloading. */
> struct offload_info {
> - bool recirc_id_shared_with_tc; /* Indicates whever tc chains will be in
> - * sync with datapath recirc ids. */
> -
> /*
> * The flow mark id assigned to the flow. If any pkts hit the flow,
> * it will be in the pkt meta data.
> */
> uint32_t flow_mark;
>
> - bool tc_modify_flow; /* Indicates tc modified the flow. */
> - 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. */
> };
>
> @@ -87,7 +81,6 @@ int netdev_flow_del(struct netdev *, const ovs_u128 *,
> struct dpif_flow_stats *);
> int netdev_init_flow_api(struct netdev *);
> void netdev_uninit_flow_api(struct netdev *);
> -uint32_t netdev_get_block_id(struct netdev *);
> int netdev_get_hw_info(struct netdev *, int);
> void netdev_set_hw_info(struct netdev *, int, int);
> bool netdev_any_oor(void);
> diff --git a/lib/netdev.c b/lib/netdev.c
> index c5182fe8f..dbf8ac574 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -154,8 +154,6 @@ netdev_initialize(void)
> netdev_register_provider(&netdev_internal_class);
> netdev_register_provider(&netdev_tap_class);
> netdev_vport_tunnel_register();
> -
> - netdev_register_flow_api_provider(&netdev_offload_tc);
> #ifdef HAVE_AF_XDP
> netdev_register_provider(&netdev_afxdp_class);
> netdev_register_provider(&netdev_afxdp_nonpmd_class);
> @@ -2455,3 +2453,13 @@ netdev_free_custom_stats_counters(struct
> netdev_custom_stats *custom_stats)
> }
> }
> }
> +
> +uint32_t
> +netdev_get_block_id(struct netdev *netdev)
> +{
> + const struct netdev_class *class = netdev->netdev_class;
> +
> + return (class->get_block_id
> + ? class->get_block_id(netdev)
> + : 0);
> +}
> diff --git a/lib/netdev.h b/lib/netdev.h
> index 63e03d72d..df56c9071 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -216,6 +216,7 @@ int netdev_set_tx_multiq(struct netdev *, unsigned int
> n_txq);
> enum netdev_pt_mode netdev_get_pt_mode(const struct netdev *);
> void netdev_set_dpif_type(struct netdev *, const char *);
> const char *netdev_get_dpif_type(const struct netdev *);
> +uint32_t netdev_get_block_id(struct netdev *);
>
> /* Packet reception. */
> int netdev_rxq_open(struct netdev *, struct netdev_rxq **, int id);
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
