[ovs-dev] [v13 05/12] dpif-netdev: Add command to switch dpif implementation.

Flavio Leitner fbl at sysclose.org
Thu Jun 24 12:26:13 UTC 2021


On Thu, Jun 24, 2021 at 11:53:34AM +0000, Ferriter, Cian wrote:
[...]
> > On Thu, Jun 17, 2021 at 05:18:18PM +0100, Cian Ferriter wrote:
> > > +
> > > +VLOG_DEFINE_THIS_MODULE(dpif_netdev_impl);
> > > +
> > > +/* Actual list of implementations goes here. */
> > > +static struct dpif_netdev_impl_info_t dpif_impls[] = {
> > > +    /* The default scalar C code implementation. */
> > > +    { .func = dp_netdev_input,
> > 
> > The '.func' is too generic. Can we rename this to something that
> > relates to 'input'?
> > 
> 
> I'll rename to 'input_func'. Does that sound good to you?

Sounds better, indeed.

[..]
> > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > > index 1f15af882..9c234ef3d 100644
> > > --- a/lib/dpif-netdev.c
> > > +++ b/lib/dpif-netdev.c
> > > @@ -479,8 +479,8 @@ static void dp_netdev_execute_actions(struct dp_netdev_pmd_thread *pmd,
> > >                                        const struct flow *flow,
> > >                                        const struct nlattr *actions,
> > >                                        size_t actions_len);
> > > -static int32_t dp_netdev_input(struct dp_netdev_pmd_thread *,
> > > -                            struct dp_packet_batch *, odp_port_t port_no);
> > > +int32_t dp_netdev_input(struct dp_netdev_pmd_thread *,
> > > +                        struct dp_packet_batch *, odp_port_t port_no);
> > 
> > 
> > All other functions around are static and this one is now part of
> > dpif-netdev-private-dpif.h which is included by
> > dpif-netdev-private-thread.h as part of dpif-netdev-private.h.
> > 
> > Perhaps fixing that header issue I reported in the first patch
> > helps to solve this too.
> > 
> 
> This function can't be static. We need it to be available in
> lib/dpif-netdev-private-dpif.c to register it as the DPIF function
> for the dpif_scalar dpif_impl. I hope that makes sense.

Sure, it can't be. What I am saying is that it is declared in two
different places. One in dpif-netdev.c and another in the header
dpif-netdev-private-dpif.h which is included as well.

Thanks,
-- 
fbl


More information about the dev mailing list