[ovs-dev] [PATCH] netdev: Dynamic per-port Flow API.
Ilya Maximets
i.maximets at ovn.org
Fri Nov 22 15:50:24 UTC 2019
On 22.11.2019 14:51, Ophir Munk wrote:
> Hi Ilya,
> Thanks for your explanations.
> We are using just one vport ("vxlan0") and still the netdev flow
> api pointer is NULL during a flow_put() call.
> Following is a root cause description.
>
> In short:
> 1. A new port->netdev named "vxlan_sys_4789" is created as part of adding the
> "vxlan0" netdev port.
> The new port->netdev is created without initializing it with flow api pointers.
> 2. We call netdev_init_flow_api(netdev) on the "vxlan0" netdev.
> 3. During flow_put we call with the port->netdev "vxlan_sys_4789", whose flow
> api pointers are NULL.
>
> In details:
> 1. For "vxlan0" interface we have a call dpif_port_add (dpif.c)
> 2. Inside this function we have a call
> dpif->dpif_class->port_add(dpif, netdev, &port_no);
> Which calls the netdev API dpif_netdev_port_add()
> 3. Inside dpif_netdev_port_add() dpif_port gets a new name: "vxlan_sys_4789":
> dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
>
> Then we call do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no) with
> the new name.
> We have a sequence of calls that eventually creates a new netdev for
> dpif_port "vxlan_sys_4789"
> do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no);
> ==> port_create(devname, type, port_no, &port);
> ==> netdev_open(devname, type, &netdev);
> ==> netdev = rc->class->alloc();
> The newly created port holds the new port->netdev and it is added to hash map:
> hmap_insert(&dp->ports, &port->node, hash_port_no(port_no));
> Please note port->netdev was not initialized with flow api.
>
> Back to dpif_port_add (dpif.c) we then have a call at the end to
> netdev_ports_insert() (file netdev_offload.c) which calls netdev_init_flow_api("vxlan0" netdev).
> The netdev parameter is the "vxlan0" netdev and not the port->netdev "vxlan_sys_4789"
> that was created.
>
> When dp_netdev_flow_add() it called (file dpif-netdev.c) we will find
> the port->netdev "vxlan_sys_4789".
> As mentioned this netdev has never been initialized with flow API.
> Therefore its flow pointers are NULL.
> The pointers were set successfully on the "vxlan0" netdev - but these pointer are not
> used during a flow_put() call.
>
> By the way for a "dpdk" type netdev this issue does not happen. It is because the dpif port
> name has the same name as the interface name.
> No new netdev is allocated. Therefore flow_put will call with the port->netdev that was
> initialized with flow api pointers.
> Hence there is no issue in this case.
>
> I would appreciate your comments on this.
>
Thanks for figuring this out!
Yes, it seems a bit strange that dpif-netdev creates new netdev instance
and maybe we could avoid that somehow. However, this unveiled the bug
which is improper usage of offloading API. Datapath should request
netdevs from the netdev-offload module with netdev_ports_get() and use
resulted netdev for netdev_flow_put(), but dpif-netdev for some reason
performs lookup in the datapath port list and that is the issue.
I prepared the patch for that:
https://patchwork.ozlabs.org/patch/1199525/
Some changes will be needed to use it along with my previous patch-set
for vport offloading, but they are straightforward.
Best regards, Ilya Maximets.
> Regards,
> Ophir
>
>> -----Original Message-----
>> From: Ilya Maximets <i.maximets at ovn.org>
>> Sent: Wednesday, November 20, 2019 7:00 PM
>> To: Ophir Munk <ophirmu at mellanox.com>; Ilya Maximets
>> <i.maximets at ovn.org>; ovs-dev at openvswitch.org
>> Cc: Ameer Mahagneh <ameerm at mellanox.com>; Roni Bar Yanai
>> <roniba at mellanox.com>; Eveline Raine <eveliner at mellanox.com>; Oz
>> Shlomo <ozsh at mellanox.com>; Eli Britstein <elibr at mellanox.com>
>> Subject: Re: [PATCH] netdev: Dynamic per-port Flow API.
>>
>> On 20.11.2019 17:52, Ophir Munk wrote:
>>> Hi Ilya,
>>> I think the problem is that the newly created netdev called
>> "vxlan_sys_4789" never goes through a netdev_init_flow_api(netdev); call.
>>
>> 'vxlan_sys_4789' is not a netdev. It's a dpif_port.
>> There is a corresponding 'netdev', but only 'ofproto' knows which one.
>>
>>> Where do you suggest adding this call?
>>>
>>> Regards,
>>> Ophir
>>>
>>>> -----Original Message-----
>>>> From: Ilya Maximets <i.maximets at ovn.org>
>>>> Sent: Tuesday, November 19, 2019 8:30 PM
>>>> To: Ophir Munk <ophirmu at mellanox.com>; Ilya Maximets
>>>> <i.maximets at ovn.org>; ovs-dev at openvswitch.org
>>>> Cc: Ameer Mahagneh <ameerm at mellanox.com>; Roni Bar Yanai
>>>> <roniba at mellanox.com>; Eveline Raine <eveliner at mellanox.com>; Oz
>>>> Shlomo <ozsh at mellanox.com>; Eli Britstein <elibr at mellanox.com>
>>>> Subject: Re: [PATCH] netdev: Dynamic per-port Flow API.
>>>>
>>>> On 19.11.2019 18:20, Ophir Munk wrote:
>>>>> Hi Ilya,
>>>>> Thanks for the patch set which adds the dpif hint inside the netdev
>> struct.
>>>> It is really helpful.
>>>>> Our goal is to have flow_put() calls on vxlan devices, using the
>>>>> existing dpdk
>>>> flow API.
>>>>> We added logic inside netdev_dpdk_flow_api_supported() to accept
>>>>> vxlan devices and indeed we see in the log:
>>>>> "netdev_offload|INFO|vxlan0: Assigned flow API 'dpdk_flow_api'".
>>>>> However, there is no flow_put() on the vxlan device.
>>>>> We see our own printouts in dpif-netdev such as:
>>>>> "dpif_netdev(dp_netdev_flow_5)|ERR|vxlan_sys_4789: calling netdev
>>>> flow_put()"
>>>>> but inside netdev_flow_put the flow_api poiner is NULL.
>>>>> Any ideas why?
>>>>
>>>> How many tunneling ports you have?
>>>> The case here is that dpif creates only one dpif_port for all the
>>>> tunnels with similar config. So, only one of these netdevs will have
>>>> flow api initialized and you need to be sure that you're using the
>>>> right one. You may find it by the datapath 'port_no' with
>> netdev_ports_get().
>>>> Just guessing, but this is the one possible reason.
>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ilya Maximets <i.maximets at ovn.org>
>>>>>> Sent: Friday, November 15, 2019 10:26 PM
>>>>>> To: Ophir Munk <ophirmu at mellanox.com>; Ilya Maximets
>>>>>> <i.maximets at ovn.org>; ovs-dev at openvswitch.org
>>>>>> Cc: Ameer Mahagneh <ameerm at mellanox.com>; Roni Bar Yanai
>>>>>> <roniba at mellanox.com>; Eveline Raine <eveliner at mellanox.com>; Oz
>>>>>> Shlomo <ozsh at mellanox.com>; Eli Britstein <elibr at mellanox.com>
>>>>>> Subject: Re: [PATCH] netdev: Dynamic per-port Flow API.
>>>>>>
>>>>>> On 10.11.2019 21:17, Ophir Munk wrote:
>>>>>>> Hi Ilya,
>>>>>>> Thanks you for taking care of this.
>>>>>>> Assigning the correct vport flow API is a critical feature for offloading.
>>>>>>>
>>>>>>> It seems hard to address all different vport configuration
>>>>>>> scenarios (kernel
>>>>>> only, userspace only or both) by just relying on the individual
>>>>>> netdev and without knowing the dpif on top.
>>>>>>
>>>>>> Yes you're right. The only module that knows the real mapping of
>>>>>> 'netdev's to 'dpif's is 'ofproto' and we need to get this
>>>>>> information
>>>> somehow.
>>>>>>
>>>>>>> Maybe can move the flow API assignment to the point where dp is
>>>>>>> actually
>>>>>> adding the port netdev and give a hint about the dp.
>>>>>>
>>>>>> Since we already have a hint from the upper layers about the
>>>>>> 'dpif_class' I'm suggesting to replace it with 'dpif_type'.
>>>>>> In pair with storing this hint inside the netdev we could have a
>>>>>> flexible system without exposing internals to higher layers or
>>>>>> trying to use higher layer from the library code.
>>>>>>
>>>>>> Please, take a look at the new version of patches:
>>>>>>
>>>>>>
>>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.
>>>>>> openvswitch.org%2Fpipermail%2Fovs-dev%2F2019-
>>>>>>
>>>>
>> November%2F364735.html&data=02%7C01%7Cophirmu%40mellanox.c
>>>>>>
>>>>
>> om%7Caba8a2cdff2342aa536b08d76a09fc61%7Ca652971c7d2e4d9ba6a4d149
>>>>>>
>>>>
>> 256f461b%7C0%7C0%7C637094463434958586&sdata=taEV4B7BA83%2Fu
>>>>>> faHoW7Y7F824SumWEnEXhVUpUON5eA%3D&reserved=0
>>>>>>
>>>>>>> It is also confusing that userspace vport names are "sys_vxlan_4789"
>>>>>>> and
>>>>>> not "usr_vxlan_4789" for example.
>>>>>>
>>>>>> Yes, this is a bit confusing, but we, probably, could not just
>>>>>> change it. At least that we'll have to rewrite a big part of
>>>>>> system tests that relies on the port names in the flow dumps.
>>>>>> We will have to duplicate them for kernel and userspace.
>>>>>> Some external management/monitoring tools could be affected too
>>>>>> because they will have to treat kernel and userspace tunneling
>>>> differently.
>>>>>>
>>>>>> Best regards, Ilya Maximets.
>>>>>>
>>>>>>> What do you think?
>>>>>>>
>>>>>>> Regards,
>>>>>>> Ophir
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Ilya Maximets <i.maximets at ovn.org>
>>>>>>>> Sent: Friday, November 8, 2019 5:57 PM
>>>>>>>> To: Ophir Munk <ophirmu at mellanox.com>; Ilya Maximets
>>>>>>>> <i.maximets at ovn.org>; ovs-dev at openvswitch.org
>>>>>>>> Cc: Ameer Mahagneh <ameerm at mellanox.com>; Roni Bar Yanai
>>>>>>>> <roniba at mellanox.com>; Eveline Raine <eveliner at mellanox.com>
>>>>>>>> Subject: Re: [PATCH] netdev: Dynamic per-port Flow API.
>>>>>>>>
>>>>>>>> On 06.11.2019 18:11, Ophir Munk wrote:
>>>>>>>>> Hi Ilya,
>>>>>>>>> A few months ago we discussed the missing functionality of
>>>>>>>>> vports
>>>>>>>> offloading under user space bridges.
>>>>>>>>> Commit [1] was added to explicitly avoid installing userspace
>>>>>>>>> vport flows (to
>>>>>>>> avoid confusion with the vport kernel).
>>>>>>>>> When reverting commit [1] - we are left with this missing
>>>> functionality.
>>>>>>>>> Applying your suggested patch netdev_vport_has_system_port()
>>>> API)
>>>>>>>> does not seem to work.
>>>>>>>>> It always fails when it tries to look for the interface name
>>>>>>>>> (say
>>>>>>>>> "vxlan10") in
>>>>>>>> the system list where vxlan interfaces are renamed (say
>>>>>> "vxlan_sys_4789").
>>>>>>>>
>>>>>>>> Hi Ophir,
>>>>>>>>
>>>>>>>> No-one ever tried to run this code, so I'm not surprised.
>>>>>>>> I could take a look at this issue on next week.
>>>>>>>>
>>>>>>>> Best regards, Ilya Maximets.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> You wrote:
>>>>>>>>>> On master, after applying dynamic flow API patch-set, we'll be
>>>>>>>>>> able to use patch that I suggested in previous mail to properly
>>>>>>>>>> handle this situation and enable vport offloading.
>>>>>>>>>
>>>>>>>>> It would be appreciated if we can resume the efforts in fixing
>>>>>>>>> this missing
>>>>>>>> functionality.
>>>>>>>>> Please advise on the next steps.
>>>>>>>>>
>>>>>>>>> [1]
>>>>>>>>> Commit 0da667e345 ("dpif-netdev: Forbid vport offloading
>>>>>>>>> attempts.")
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Ilya Maximets <i.maximets at samsung.com>
>>>>>>>>>> Sent: Monday, May 13, 2019 3:33 PM
>>>>>>>>>> To: Ophir Munk <ophirmu at mellanox.com>; ovs-
>>>>>> dev at openvswitch.org
>>>>>>>>>> Cc: Ian Stokes <ian.stokes at intel.com>; Flavio Leitner
>>>>>>>>>> <fbl at sysclose.org>; Kevin Traynor <ktraynor at redhat.com>; Roni
>>>> Bar
>>>>>>>>>> Yanai <roniba at mellanox.com>; Finn Christensen
>>>> <fc at napatech.com>;
>>>>>>>> Ben
>>>>>>>>>> Pfaff <blp at ovn.org>; Simon Horman
>>>>>> <simon.horman at netronome.com>;
>>>>>>>> Olga
>>>>>>>>>> Shern <olgas at mellanox.com>; Asaf Penso
>> <asafp at mellanox.com>;
>>>> Oz
>>>>>>>>>> Shlomo <ozsh at mellanox.com>; Majd Dibbiny
>>>> <majd at mellanox.com>
>>>>>>>>>> Subject: Re: [PATCH] netdev: Dynamic per-port Flow API.
>>>>>>>>>>
>>>>>>>>>> On 13.05.2019 14:41, Ophir Munk wrote:
>>>>>>>>>>> Hi Ilya,
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: Ilya Maximets <i.maximets at samsung.com>
>>>>>>>>>>>> Sent: Monday, May 13, 2019 12:22 PM
>>>>>>>>>>>> To: Ophir Munk <ophirmu at mellanox.com>; ovs-
>>>>>>>> dev at openvswitch.org
>>>>>>>>>>>> Cc: Ian Stokes <ian.stokes at intel.com>; Flavio Leitner
>>>>>>>>>>>> <fbl at sysclose.org>; Kevin Traynor <ktraynor at redhat.com>;
>> Roni
>>>>>> Bar
>>>>>>>>>>>> Yanai <roniba at mellanox.com>; Finn Christensen
>>>>>> <fc at napatech.com>;
>>>>>>>>>> Ben
>>>>>>>>>>>> Pfaff <blp at ovn.org>; Simon Horman
>>>>>>>> <simon.horman at netronome.com>;
>>>>>>>>>> Olga
>>>>>>>>>>>> Shern <olgas at mellanox.com>; Asaf Penso
>>>> <asafp at mellanox.com>;
>>>>>> Oz
>>>>>>>>>>>> Shlomo <ozsh at mellanox.com>
>>>>>>>>>>>> Subject: Re: [PATCH] netdev: Dynamic per-port Flow API.
>>>>>>>>>>>>
>>>>>>>>>>>> On 09.05.2019 1:59, Ophir Munk wrote:
>>>>>>>>>>>>> Hi Ilya,
>>>>>>>>>>>>>
>>>>>>>>>>> ...
>>>>>>>>>>>>> I am recreating this scenario in my setup.
>>>>>>>>>>>>
>>>>>>>>>>>> I see. Yes, you're right. And I think that this case could be
>>>>>>>>>>>> reproduced on current master without any patches. So, it's a
>>>>>>>>>>>> bug that we
>>>>>>>>>> need to fix.
>>>>>>>>>>>> Otherwise userspace datapath will try to offload its flows to
>>>>>>>>>>>> the unrelated system interfaces. For now we could just forbid
>>>>>>>>>>>> offloading to vports in dpif- netdev. I'll prepare the patch.
>>>>>>>>>>>> This fix also should be
>>>>>>>>>> backported.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> We need a way to enable offloading of vports in dpif-netdev.
>>>>>>>>>>> So if you can address It with your new patch - that would be
>>>> appreciated.
>>>>>>>>>>
>>>>>>>>>> I'm suggesting disabling because it'll be easy to backport to
>>>>>>>>>> older
>>>>>>>> branches.
>>>>>>>>>> On master, after applying dynamic flow API patch-set, we'll be
>>>>>>>>>> able to use patch that I suggested in previous mail to properly
>>>>>>>>>> handle this situation and enable vport offloading.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> This patch series is important but one of its main goals is
>>>>>>>>>>>>> to enable different
>>>>>>>>>>>> offloads for different vports under Kernel/userspace.
>>>>>>>>>>>>> Can you please advise how this goal can be achieved?
>>>>>>>>>>>>
>>>>>>>>>>>> It looks like there is no way to distinguish system and
>>>>>>>>>>>> userspace vports by looking only on netdev. We should check
>>>> with dpif type.
>>>>>>>>>>>
>>>>>>>>>>> Agreed. But then looking at your sample patch below you are
>>>>>>>>>>> basing your decision on the existence of system port (and not
>>>>>>>>>>> on
>>>> dpif type).
>>>>>>>>>>
>>>>>>>>>> Right now 'ifindex' is used for checking, so this is equally racy.
>>>>>>>>>>
>>>>>>>>>>> I think it is risky because
>>>>>>>>>>> you are dependent on the order of operations: Creation of the
>>>>>>>>>>> system port
>>>>>>>>>> versus checking for system port existence.
>>>>>>>>>>> Which was first (under different scenarios)?
>>>>>>>>>>
>>>>>>>>>> Actually, port creation and checking will always happen in the
>>>>>>>>>> same thread context, so creation and checking will be serialized.
>>>>>>>>>> Kernel should guarantee the port existence. No races here.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> What do you say about the following patch:
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c int
>>>>>>>>>>> netdev_ports_insert(struct netdev *netdev, const struct
>>>>>>>>>>> dpif_class *dpif_class,
>>>>>>>>>>> struct dpif_port *dpif_port) @@ -547,8
>>>>>>>>>>> +555,9 @@ netdev_ports_insert(struct netdev *netdev, const
>>>>>>>>>>> +struct
>>>>>>>>>>> dpif_class *dpif_class,
>>>>>>>>>>> netdev_ports_hash(dpif_port->port_no, dpif_class));
>>>>>>>>>>> hmap_insert(&ifindex_to_port, &data->ifindex_node,
>> ifindex);
>>>>>>>>>>> ovs_mutex_unlock(&netdev_hmap_mutex);
>>>>>>>>>>>
>>>>>>>>>>> - netdev_init_flow_api(netdev);
>>>>>>>>>>> + netdev_init_flow_api(netdev, dpif_class->type); /* Pass
>>>>>>>>>>> + class type "netdev" or "syste" */
>>>>>>>>>>>
>>>>>>>>>>> return 0;
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> It's not a full solution. It is just a hint how to pass the
>>>>>>>>>>> dpif type ("netdev" or
>>>>>>>>>> "system") when initializing the flow api.
>>>>>>>>>>> We can use the dpif type when initializing vport offload.
>>>>>>>>>>
>>>>>>>>>> This is, actually, the first thing I tried. However, this
>>>>>>>>>> requires exposing the internals of 'dpif-provider', which is
>>>>>>>>>> bad from the point of
>>>>>>>> code architecture.
>>>>>>>>>>
>>>>>>>>>> For the below patch code, I missed actual port requesting from
>>>>>>>>>> the
>>>>>>>> datapath.
>>>>>>>>>> Following incremental needed:
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index
>>>>>>>>>> 82943c102..1c88a91ed 100644
>>>>>>>>>> --- a/lib/netdev-vport.c
>>>>>>>>>> +++ b/lib/netdev-vport.c
>>>>>>>>>> @@ -124,13 +124,28 @@
>> netdev_vport_class_get_dpif_port(const
>>>>>> struct
>>>>>>>>>> netdev_class *class) bool netdev_vport_has_system_port(const
>>>>>>>>>> struct netdev *netdev) {
>>>>>>>>>> - bool found;
>>>>>>>>>> + bool found = false;
>>>>>>>>>> + const char *name;
>>>>>>>>>> + const char *type = "system";
>>>>>>>>>> struct sset names = SSET_INITIALIZER(&names);
>>>>>>>>>>
>>>>>>>>>> ovs_assert(is_vport_class(netdev_get_class(netdev)));
>>>>>>>>>>
>>>>>>>>>> - dp_enumerate_names("system", &names);
>>>>>>>>>> - found = sset_contains(&names, netdev_get_name(netdev));
>>>>>>>>>> + dp_enumerate_names(type, &names);
>>>>>>>>>> + SSET_FOR_EACH (name, &names) {
>>>>>>>>>> + struct dpif *dpifp = NULL;
>>>>>>>>>> +
>>>>>>>>>> + if (dpif_open(name, type, &dpifp)) {
>>>>>>>>>> + continue;
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + found = dpif_port_exists(dpifp,
>>>>>>>>>> + netdev_get_name(netdev));
>>>>>>>>>> +
>>>>>>>>>> + dpif_close(dpifp);
>>>>>>>>>> + if (found) {
>>>>>>>>>> + break;
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>> sset_destroy(&names);
>>>>>>>>>>
>>>>>>>>>> return found;
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> Something like this (not tested):
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/lib/netdev-offload-dpdk.c
>>>>>>>>>>>> b/lib/netdev-offload-dpdk.c index 01e900461..b7b0616ec
>> 100644
>>>>>>>>>>>> --- a/lib/netdev-offload-dpdk.c
>>>>>>>>>>>> +++ b/lib/netdev-offload-dpdk.c
>>>>>>>>>>>> @@ -22,6 +22,7 @@
>>>>>>>>>>>> #include "dpif-netdev.h"
>>>>>>>>>>>> #include "netdev-offload-provider.h"
>>>>>>>>>>>> #include "netdev-provider.h"
>>>>>>>>>>>> +#include "netdev-vport.h"
>>>>>>>>>>>> #include "openvswitch/match.h"
>>>>>>>>>>>> #include "openvswitch/vlog.h"
>>>>>>>>>>>> #include "packets.h"
>>>>>>>>>>>> @@ -752,6 +753,13 @@ netdev_offload_dpdk_flow_del(struct
>>>>>> netdev
>>>>>>>>>>>> *netdev, const ovs_u128 *ufid, static int
>>>>>>>>>>>> netdev_offload_dpdk_init_flow_api(struct netdev *netdev) {
>>>>>>>>>>>> + if (netdev_vport_is_vport_class(netdev->netdev_class)
>>>>>>>>>>>> + && netdev_vport_has_system_port(netdev)) {
>>>>>>>>>>>> + VLOG_DBG("%s: vport has backing system interface.
>>>>>> Skipping.",
>>>>>>>>>>>> + netdev_get_name(netdev));
>>>>>>>>>>>> + return EOPNOTSUPP;
>>>>>>>>>>>> + }
>>>>>>>>>>>> +
>>>>>>>>>>>> return netdev_dpdk_flow_api_supported(netdev) ? 0 :
>>>>>>>>>>>> EOPNOTSUPP; }
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/lib/netdev-offload-tc.c
>>>>>>>>>>>> b/lib/netdev-offload-tc.c index
>>>>>>>>>>>> 498aae369..e4d121ed7 100644
>>>>>>>>>>>> --- a/lib/netdev-offload-tc.c
>>>>>>>>>>>> +++ b/lib/netdev-offload-tc.c
>>>>>>>>>>>> @@ -31,6 +31,7 @@
>>>>>>>>>>>> #include "netdev-linux.h"
>>>>>>>>>>>> #include "netdev-offload-provider.h"
>>>>>>>>>>>> #include "netdev-provider.h"
>>>>>>>>>>>> +#include "netdev-vport.h"
>>>>>>>>>>>> #include "netlink.h"
>>>>>>>>>>>> #include "netlink-socket.h"
>>>>>>>>>>>> #include "odp-netlink.h"
>>>>>>>>>>>> @@ -1560,6 +1561,13 @@ netdev_tc_init_flow_api(struct
>> netdev
>>>>>>>>>> *netdev)
>>>>>>>>>>>> int ifindex;
>>>>>>>>>>>> int error;
>>>>>>>>>>>>
>>>>>>>>>>>> + if (netdev_vport_is_vport_class(netdev->netdev_class)
>>>>>>>>>>>> + && !netdev_vport_has_system_port(netdev)) {
>>>>>>>>>>>> + VLOG_DBG("%s: vport has no backing system interface.
>>>>>>>> Skipping.",
>>>>>>>>>>>> + netdev_get_name(netdev));
>>>>>>>>>>>> + return EOPNOTSUPP;
>>>>>>>>>>>> + }
>>>>>>>>>>>> +
>>>>>>>>>>>> ifindex = netdev_get_ifindex(netdev);
>>>>>>>>>>>> if (ifindex < 0) {
>>>>>>>>>>>> VLOG_INFO("init: failed to get ifindex for %s: %s",
>>>>>>>>>>>> diff --git a/lib/netdev- vport.c b/lib/netdev-vport.c index
>>>>>>>>>>>> 92a256af1..82943c102 100644
>>>>>>>>>>>> --- a/lib/netdev-vport.c
>>>>>>>>>>>> +++ b/lib/netdev-vport.c
>>>>>>>>>>>> @@ -44,6 +44,7 @@
>>>>>>>>>>>> #include "simap.h"
>>>>>>>>>>>> #include "smap.h"
>>>>>>>>>>>> #include "socket-util.h"
>>>>>>>>>>>> +#include "sset.h"
>>>>>>>>>>>> #include "unaligned.h"
>>>>>>>>>>>> #include "unixctl.h"
>>>>>>>>>>>> #include "openvswitch/vlog.h"
>>>>>>>>>>>> @@ -120,6 +121,21 @@
>> netdev_vport_class_get_dpif_port(const
>>>>>>>> struct
>>>>>>>>>>>> netdev_class *class)
>>>>>>>>>>>> return is_vport_class(class) ?
>>>>>>>>>>>> vport_class_cast(class)->dpif_port : NULL; }
>>>>>>>>>>>>
>>>>>>>>>>>> +bool
>>>>>>>>>>>> +netdev_vport_has_system_port(const struct netdev
>> *netdev) {
>>>>>>>>>>>> + bool found;
>>>>>>>>>>>> + struct sset names = SSET_INITIALIZER(&names);
>>>>>>>>>>>> +
>>>>>>>>>>>> + ovs_assert(is_vport_class(netdev_get_class(netdev)));
>>>>>>>>>>>> +
>>>>>>>>>>>> + dp_enumerate_names("system", &names);
>>>>>>>>>>>> + found = sset_contains(&names,
>> netdev_get_name(netdev));
>>>>>>>>>>>> + sset_destroy(&names);
>>>>>>>>>>>> +
>>>>>>>>>>>> + return found;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>> const char *
>>>>>>>>>>>> netdev_vport_get_dpif_port(const struct netdev *netdev,
>>>>>>>>>>>> char namebuf[], size_t bufsize)
>>>>>>>>>>>> diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h index
>>>>>>>>>>>> 9d756a265..4be1b92ec 100644
>>>>>>>>>>>> --- a/lib/netdev-vport.h
>>>>>>>>>>>> +++ b/lib/netdev-vport.h
>>>>>>>>>>>> @@ -40,6 +40,7 @@ void netdev_vport_inc_tx(const struct
>>>> netdev
>>>>>> *,
>>>>>>>>>>>> const struct dpif_flow_stats *);
>>>>>>>>>>>>
>>>>>>>>>>>> bool netdev_vport_is_vport_class(const struct netdev_class
>>>>>>>>>>>> *);
>>>>>>>>>>>> +bool netdev_vport_has_system_port(const struct netdev *);
>>>>>>>>>>>> const char *netdev_vport_class_get_dpif_port(const struct
>>>>>>>>>>>> netdev_class *);
>>>>>>>>>>>>
>>>>>>>>>>>> #ifndef _WIN32
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>> I'll format this as a proper patch and send along with patch
>>>>>>>>>>>> for enabling offloading for ports without correct ifindex.
>>>>>>>>>>>>
>>>>>>>>>>>> Best regards, Ilya Maximets.
>>>>>>>>>>>
>>>>>>>>>>> Best regards,
>>>>>>>>>>> Ophir
>>>>>>>>>>>
More information about the dev
mailing list