On 20/01/2025 14:57, Eelco Chaudron wrote: > > > On 15 Jan 2025, at 10:18, Roi Dayan via dev wrote: > >> Use this instead of directly using dpif_is_netdev() from dpif-netdev. >> Return true in dpif-netdev. > > Not sure if we need an API for this, or maybe I should say, what is the > definition of userspace? If this means anything but the kernel, we could > probably use !dpif_is_netlink(). > > But I guess, the better question would be, what would we use this call for? > If it’s similar to the existing use cases, we are probably better off by > adding a features API to dpif. > > Thoughts? > > Cheers, > > Eelco
Hi, The meaning of dpif is a userspace means it needs to handle tunneling and other stuff that kernel dpif doesn't need as the kernel does it for it. So while !dpif_is_netlink() is even more correct and better than current dpif_is_netdev() there was also a point here that dpif.c is not familiar with a specific dpif and not accessing a specific dpif function. I think introducing a features API to dpif could be an overkill at this point. So if future dpifs might need something specific other than handled by kernel or not I think this can be done at a later time. Thanks, Roi > >> Signed-off-by: Roi Dayan <[email protected]> >> Acked-by: Eli Britstein <[email protected]> >> --- >> lib/dpif-netdev.c | 9 ++++++++- >> lib/dpif-netdev.h | 2 -- >> lib/dpif-netlink.c | 1 + >> lib/dpif-provider.h | 3 +++ >> lib/dpif.c | 14 ++++++++++---- >> 5 files changed, 22 insertions(+), 7 deletions(-) >> >> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >> index 2a529f272d16..8dd0f8c2b2b7 100644 >> --- a/lib/dpif-netdev.c >> +++ b/lib/dpif-netdev.c >> @@ -686,7 +686,7 @@ pmd_thread_ctx_time_update(struct dp_netdev_pmd_thread >> *pmd) >> } >> >> /* Returns true if 'dpif' is a netdev or dummy dpif, false otherwise. */ >> -bool >> +static bool >> dpif_is_netdev(const struct dpif *dpif) >> { >> return dpif->dpif_class->open == dpif_netdev_open; >> @@ -9995,6 +9995,12 @@ dpif_netdev_bond_stats_get(struct dpif *dpif, >> uint32_t bond_id, >> return 0; >> } >> >> +static bool >> +dpif_netdev_is_userspace(void) >> +{ >> + return true; >> +} >> + >> const struct dpif_class dpif_netdev_class = { >> "netdev", >> true, /* cleanup_required */ >> @@ -10084,6 +10090,7 @@ const struct dpif_class dpif_netdev_class = { >> NULL, /* cache_get_name */ >> NULL, /* cache_get_size */ >> NULL, /* cache_set_size */ >> + dpif_netdev_is_userspace, >> }; >> >> static void >> diff --git a/lib/dpif-netdev.h b/lib/dpif-netdev.h >> index 6db6ed2e2128..3323eb0204af 100644 >> --- a/lib/dpif-netdev.h >> +++ b/lib/dpif-netdev.h >> @@ -33,8 +33,6 @@ extern "C" { >> * headers to be aligned on a 4-byte boundary. */ >> enum { DP_NETDEV_HEADROOM = 2 + VLAN_HEADER_LEN }; >> >> -bool dpif_is_netdev(const struct dpif *); >> - >> #define NR_QUEUE 1 >> #define NR_PMD_THREADS 1 >> >> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c >> index 84e2bd8eaf58..5dbe847f61b0 100644 >> --- a/lib/dpif-netlink.c >> +++ b/lib/dpif-netlink.c >> @@ -4589,6 +4589,7 @@ const struct dpif_class dpif_netlink_class = { >> dpif_netlink_cache_get_name, >> dpif_netlink_cache_get_size, >> dpif_netlink_cache_set_size, >> + NULL, /* is_userspace */ >> }; >> >> static int >> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h >> index 520e21e68db2..e8a8d36a80c5 100644 >> --- a/lib/dpif-provider.h >> +++ b/lib/dpif-provider.h >> @@ -683,6 +683,9 @@ struct dpif_class { >> >> /* Set cache size. */ >> int (*cache_set_size)(struct dpif *dpif, uint32_t level, uint32_t size); >> + >> + /* Return if dpif is a userspace datapath. */ >> + bool (*is_userspace)(void); >> }; >> >> extern const struct dpif_class dpif_netlink_class; >> diff --git a/lib/dpif.c b/lib/dpif.c >> index a064f717f1a6..e281e893c98e 100644 >> --- a/lib/dpif.c >> +++ b/lib/dpif.c >> @@ -1930,17 +1930,23 @@ log_flow_get_message(const struct dpif *dpif, >> } >> } >> >> +static bool >> +dpif_is_userspace(const struct dpif *dpif) >> +{ >> + return dpif->dpif_class->is_userspace && >> dpif->dpif_class->is_userspace(); >> +} >> + >> bool >> dpif_supports_tnl_push_pop(const struct dpif *dpif) >> { >> - return dpif_is_netdev(dpif); >> + return dpif_is_userspace(dpif); >> } >> >> bool >> dpif_may_support_explicit_drop_action(const struct dpif *dpif) >> { >> /* TC does not support offloading this action. */ >> - return dpif_is_netdev(dpif) || !netdev_is_flow_api_enabled(); >> + return dpif_is_userspace(dpif) || !netdev_is_flow_api_enabled(); >> } >> >> bool >> @@ -1950,14 +1956,14 @@ dpif_supports_lb_output_action(const struct dpif >> *dpif) >> * Balance-tcp optimization is currently supported in netdev >> * datapath only. >> */ >> - return dpif_is_netdev(dpif); >> + return dpif_is_userspace(dpif); >> } >> >> bool >> dpif_may_support_psample(const struct dpif *dpif) >> { >> /* Userspace datapath does not support this action. */ >> - return !dpif_is_netdev(dpif); >> + return !dpif_is_userspace(dpif); >> } >> >> /* Meters */ >> -- >> 2.21.0 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
