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

Ferriter, Cian cian.ferriter at intel.com
Thu Jun 24 12:57:32 UTC 2021


Hi Flavio,

Thanks for clarifying. My response is inline.

Cian

> -----Original Message-----
> From: Flavio Leitner <fbl at sysclose.org>
> Sent: Thursday 24 June 2021 13:26
> To: Ferriter, Cian <cian.ferriter at intel.com>
> Cc: ovs-dev at openvswitch.org; i.maximets at ovn.org
> Subject: Re: [ovs-dev] [v13 05/12] dpif-netdev: Add command to switch dpif implementation.
> 
> 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.
> 

My apologies, I understand now. I'll fix this. I'm going to remove the lib/dpif-netdev.c declaration of dp_netdev_input().

> Thanks,
> --
> fbl


More information about the dev mailing list