[ovs-dev] [RFC PATCH v4 3/6] dpif-netlink: Probe for out-of-tree datapath.

Eric Garver e at erig.me
Fri Feb 3 20:56:47 UTC 2017


On Thu, Feb 02, 2017 at 02:50:01PM -0800, Joe Stringer wrote:
> On 18 January 2017 at 11:45, Eric Garver <e at erig.me> wrote:
> > For out-of-tree datapath, only try genetlink/compat.
> > For in-tree kernel datapath, try rtnetlink then genetlink.
> >
> > Signed-off-by: Eric Garver <e at erig.me>
> > ---
> >  lib/dpif-netlink.c | 35 ++++++++++++++++++++++++++++++++---
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index e6459f358989..769806eadbf1 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -210,6 +210,11 @@ static int ovs_packet_family;
> >   * Initialized by dpif_netlink_init(). */
> >  static unsigned int ovs_vport_mcgroup;
> >
> > +/* rtnetlink information for OVS.
> > + *
> > + * Initialized by dpif_netlink_init(). */
> > +static bool ovs_datapath_out_of_tree = false;
> 
> Perhaps in the comment briefly mention that if this is true, devices
> are created using OVS netlink and if it's false devices are created
> using NETLINK_ROUTE / as LWT?

I'll buff the comment to explain this.

> > +
> >  static int dpif_netlink_init(void);
> >  static int open_dpif(const struct dpif_netlink_dp *, struct dpif **);
> >  static uint32_t dpif_netlink_port_get_pid(const struct dpif *,
> > @@ -1014,11 +1019,13 @@ dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev,
> >                        odp_port_t *port_nop)
> >  {
> >      struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> > -    int error;
> > +    int error = EOPNOTSUPP;
> >
> >      fat_rwlock_wrlock(&dpif->upcall_lock);
> > -    error = dpif_netlink_port_create_and_add(dpif, netdev, port_nop);
> > -    if (error == EOPNOTSUPP) {
> > +    if (!ovs_datapath_out_of_tree) {
> > +        error = dpif_netlink_port_create_and_add(dpif, netdev, port_nop);
> > +    }
> > +    if (error) {
> >          error = dpif_netlink_port_add_compat(dpif, netdev, port_nop);
> >      }
> >      fat_rwlock_unlock(&dpif->upcall_lock);
> > @@ -2503,6 +2510,7 @@ dpif_netlink_init(void)
> >  {
> >      static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> >      static int error;
> > +    struct netdev *netdev = NULL;
> 
> This can be in the #ifdef __linux__; otherwise you have an unused
> variable on other platforms.

Thanks for catching. I'll address that.

> >
> >      if (ovsthread_once_start(&once)) {
> >          error = nl_lookup_genl_family(OVS_DATAPATH_FAMILY,
> > @@ -2526,6 +2534,27 @@ dpif_netlink_init(void)
> >              error = nl_lookup_genl_mcgroup(OVS_VPORT_FAMILY, OVS_VPORT_MCGROUP,
> >                                             &ovs_vport_mcgroup);
> >          }
> > +#ifdef __linux__
> > +        if (!error) {
> > +            error = netdev_open("ovs-system-probe", "geneve", &netdev);
> > +            if (!error) {
> > +                error = netdev_geneve_create_kind(netdev, "ovs_geneve");
> > +                if (error != EOPNOTSUPP) {
> > +                    if (!error) {
> > +                        char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> > +                        const char *dp_port;
> > +
> > +                        dp_port = netdev_vport_get_dpif_port(netdev, namebuf,
> > +                                                             sizeof namebuf);
> > +                        netdev_geneve_destroy(dp_port);
> > +                    }
> > +                    ovs_datapath_out_of_tree = true;
> > +                }
> > +                netdev_close(netdev);
> > +                error = 0;
> > +            }
> > +        }
> > +#endif
> 
> Geneve isn't added yet, so this patch introduces build breakage.

Oops! I rearranged the patches before submitting. I'll move this patch
to where I originally had it, after adding the newlink support.

> This doesn't look like it matches the most recent discussion on this topic:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327476.html

Correct. I opted not to do the LWT probe on init. We have to verify the
device after creation anyways, which includes checking for LWT. It
didn't seem like probing for LWT at init gained anything.


More information about the dev mailing list