On 15 Oct 2025, at 16:59, Eli Britstein wrote:
> Introduce generic API for netdev-class to apply global other_config > configuration and status update as a pre-step towards generic library > (dpdk for now) initialization and status updates. Hi Eli, Thanks for the series. I did a quick review of the first patch, and I think it would be good to get some clarification on the OVSDB API being propagated down to netdev (see comment below). As Ilya is already copied, I hope he can shed some light on this also. Cheers, Eelco > Signed-off-by: Eli Britstein <[email protected]> > --- > lib/netdev-provider.h | 12 ++++++++++++ > lib/netdev.c | 28 ++++++++++++++++++++++++++++ > lib/netdev.h | 3 +++ > 3 files changed, 43 insertions(+) > > diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h > index 5ae379469..afbb1912d 100644 > --- a/lib/netdev-provider.h > +++ b/lib/netdev-provider.h > @@ -35,6 +35,8 @@ extern "C" { > #endif > > struct netdev_tnl_build_header_params; > +struct ovsrec_open_vswitch; > + > #define NETDEV_NUMA_UNSPEC OVS_NUMA_UNSPEC > > enum netdev_ol_flags { > @@ -290,6 +292,16 @@ struct netdev_class { > * function is used to implement different classes. */ > void (*wait)(const struct netdev_class *netdev_class); > > + /* Applies global other_config settings. Maybe change this to: Pass custom configuration options to the netdev class’s implementation. > + * > + * 'netdev_class' points to the class. It is useful in case the same > + * function is used to implement different classes. */ > + int (*set_other_config)(const struct netdev_class *netdev_class, > + const struct smap *other_config); We already have set_config, which does the same thing but at the netdev level. Should we use a similar name, but with the distinction that it’s at the class level? class_set_config()? > + /* Set relevant status in 'cfg'. */ We definitely need some comments explaining what this function actually does. > + void (*class_status)(const struct ovsrec_open_vswitch *cfg); I think the name should be more meaningful. What about class_ovsdb_update() or class_ovsdb_status_update()? In addition, this is also a bit of an odd API. The OVSDB is not exposed to netdevs at all (we use the smap), and now we’re adding this as a reverse dependency. Shouldn’t we have some kind of smap interface here as well? Ilya, thoughts? > + > /* ## ---------------- ## */ > /* ## netdev Functions ## */ > /* ## ---------------- ## */ > diff --git a/lib/netdev.c b/lib/netdev.c > index df5b35232..bc8406966 100644 > --- a/lib/netdev.c > +++ b/lib/netdev.c > @@ -210,6 +210,34 @@ netdev_wait(void) > } > } > > +void > +netdev_set_other_config(const struct smap *other_config) Inline with the above; netdev_class_set_config()? > + OVS_EXCLUDED(netdev_mutex) > +{ > + netdev_initialize(); > + > + struct netdev_registered_class *rc; > + CMAP_FOR_EACH (rc, cmap_node, &netdev_classes) { > + if (rc->class->set_other_config) { > + rc->class->set_other_config(rc->class, other_config); > + } > + } > +} > + > +void > +netdev_class_status(const struct ovsrec_open_vswitch *cfg) netdev_class_ovsdb_update() or netdev_class_ovsdb_status_update(). > + OVS_EXCLUDED(netdev_mutex) > +{ > + netdev_initialize(); > + > + struct netdev_registered_class *rc; > + CMAP_FOR_EACH (rc, cmap_node, &netdev_classes) { > + if (rc->class->class_status) { > + rc->class->class_status(cfg); > + } > + } > +} > + > static struct netdev_registered_class * > netdev_lookup_class(const char *type) > { > diff --git a/lib/netdev.h b/lib/netdev.h > index 63e03d72d..febc77f12 100644 > --- a/lib/netdev.h > +++ b/lib/netdev.h > @@ -70,6 +70,7 @@ struct in6_addr; > struct smap; > struct sset; > struct ovs_action_push_tnl; > +struct ovsrec_open_vswitch; > > enum netdev_pt_mode { > /* Unknown mode. The netdev is not configured yet. */ > @@ -178,6 +179,8 @@ struct netdev_tunnel_config { > > void netdev_run(void); > void netdev_wait(void); > +void netdev_set_other_config(const struct smap *); > +void netdev_class_status(const struct ovsrec_open_vswitch *); > > void netdev_enumerate_types(struct sset *types); > bool netdev_is_reserved_name(const char *name); > -- > 2.34.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
