Hi Ian,
Please see inline.

> -----Original Message-----
> From: Stokes, Ian <[email protected]>
> Sent: Thursday, January 17, 2019 6:15 PM
> To: Stokes, Ian <[email protected]>; Ophir Munk
> <[email protected]>; Kevin Traynor <[email protected]>; ovs-
> [email protected]
> Cc: Ilya Maximets <[email protected]>
> Subject: RE: [PATCH v5] netdev-dpdk: support port representors
> 
> > > Hi Ian,
> > >
> > > > -----Original Message-----
> > > > From: Stokes, Ian <[email protected]>
> > > > Sent: Wednesday, January 16, 2019 12:37 AM
> > > > To: Kevin Traynor <[email protected]>; Ophir Munk
> > > > <[email protected]>; [email protected]
> > > > Cc: Olga Shern <[email protected]>; Ilya Maximets
> > > > <[email protected]>
> > > > Subject: RE: [PATCH v5] netdev-dpdk: support port representors
> > > >
> > > > > On 01/15/2019 09:47 AM, Ophir Munk wrote:
> > > > > > Dpdk port representors were introduced in dpdk versions 18.xx.
> > > > > > Prior to port representors there was a one-to-one relationship
> > > > > > between an rte device (e.g. PCI bus) and an eth device
> > > > > > (referenced as dpdk port id in OVS). With port representors
> > > > > > the relationship becomes one-to-many rte device to eth devices.
> > > > > > For example in [3] there are two devices (representors) using
> > > > > > the same PCI physical address 0000:08:00.0:
> > > > > > "0000:08:00.0,representor=[3]" and "0000:08:00.0,representor=[5]".
> > > > > > This commit handles the new one-to-many relationship. For
> > > > > > example, when one of the device port representors in [3] is
> > > > > > closed - the PCI bus cannot be detached until the other device
> > > > > > port representor is closed as well. OVS remains backward
> > > > > > compatible by supporting dpdk legacy PCI ports which do not
> > include port representors.
> > > > > > Dpdk port representors related commits are listed in [1]. Dpdk
> > > > > > port representors documentation appears in [2]. A sample
> > > > > > configuration which uses two representors ports (the output of
> > > > > > "ovs-
> > > vsctl show"
> > > > > > command) is shown in [3].
> > > > > >
> > > > >
> > > > > Hi Ophir, I had a scan through and there isn't any
> > > > > documentation/examples for this outside the commit message. I
> > > > > think a user would need something basic, or at least reference
> > > > > to know that this
> > > > exists and how to use it.
> > > >
> > > > +1, although I can confirm the backwards compatibility with the
> > > > +legacy pci
> > > > port methodology I'm seeing issues around the use of representors
> > > > which I'm not entirely sure are user configuration related or
> > > > specific to the patch implementation, will need more time to
> > > > investigate and
> > > confirm.
> > > >
> > > > Ian
> > >
> > > Can you please inform what exact issues you are seeing with
> > representors?
> >
> > Hi Ophir, in my case trying to add an i40e port representor was
> > failing with invalid port.
> >
> > I want to confirm this is not an issue in DPDK, or a configuration
> > issue on my side, or that it's just not supported (although I would
> > have thought it was).
> >
> > > What is the configuration?
> > PF I40e address with 05:00.0 is bound to igb_uio.
> > VF then created for PF with 'echo 1 >
> > /sys/bus/pci/devices/0000\:05\:00.0/max_vfs'.
> > VF is created with address 05:02.0
> > VF then bound to igb_uio driver.
> > OVS DPDK then stated (no new commands are used here when launching).
> > Add bridge br0
> > Attempt to add VF with port representor, use the PF address and
> > representor of VF
> >
> > ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
> > options:dpdk-devargs=0000:05:00.0,representor=[0]
> >
> > Possibly it's above where I'm going wrong. Either on the address
> > passed, or the representor value used (though I've tried a range of
> > represntor values besides 0, no luck, how do you typically identify
> > the value?)
> >
> > > What is the NIC under test?
> > The NIC is Intel X710 (Ethernet Controller X710 for 10GbE SFP+ 1572).
> > The VF created is recognized as Ethernet Virtual Function 700 Series
> > 154c
> >
> > > What is the expected result?
> > I was attempting to see if the port representor could be used to add a
> > VF for the i40e PF.
> > There is no issue if I add the VF using the legacy method ' ovs-vsctl
> > add- port br0 dpdk0 -- set Interface dpdk0 type=dpdk options:dpdk-
> > devargs=0000:05:02.0'.
> >
> > > What is the actual result?
> >
> > 2019-01-16T10:18:38Z|00062|dpdk|INFO|EAL: PCI device 0000:05:00.0 on
> > NUMA socket 0
> > 2019-01-16T10:18:38Z|00063|dpdk|ERR|EAL: Failed to attach device on
> > primary process 2019-01-16T10:18:38Z|00064|netdev_dpdk|WARN|Error
> > attaching device '0000:05:00.0,representor=[0]' to DPDK
> > 2019-01-16T10:18:38Z|00065|netdev|WARN|dpdk0: could not set
> > configuration (Invalid argument)
> > 2019-01-16T10:18:38Z|00066|dpdk|ERR|Invalid port_id=32
> > >
> > > Without those details I do not know how the "issues"  can be addressed.
> >
> > As I said this could be a user configuration issue, I spoke with Awal
> > co- authored the i40e port representor code which is how I arrived at
> > the commands above but these could be wrong.
> 
> Hi Ophir,
> 
> Quick update on this. I've managed to get the port representors working to an
> extent for i40e and ixgbe devices with this patch but there are a few issues
> causing failures.
> 
> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk options:dpdk-
> devargs=0000:05:00.0,representor=[0]
> 
> will succeed if both the PF and the VF are hotplugged to the IGB_UIO driver
> after OVS has launched.
> 
> However it will fail if PF has already been bound to the IGB_UIO driver before
> launching OVS or after being probed by a previous call to add another
> representor that shares the same PF.
> 
> This has to do with the logic around netdev_dpdk_probe_port_by_devargs().
> 
> The port_id in this will initially be set to INVALID as it will not be found 
> as part
> of the following iteration
> 
>     RTE_ETH_FOREACH_MATCHING_DEV (port_id, devargs, &iterator) {
>         /* If a break is done - must call rte_eth_iterator_cleanup. */
>         rte_eth_iterator_cleanup(&iterator);
>         break;
>     }
> 
> This makes sense as it will not have been added as a DPDK device yet in both
> hotplug and non hotplug cases.
> 
> It could be the case that the device was not probed yet, in which it is probed
> and port ID checked again.
> 
>     /* The port may be invalid if it was not probed */
>     if (port_id == DPDK_ETH_PORT_ID_INVALID) {
>         if (rte_dev_probe(devargs)) {
>             port_id = DPDK_ETH_PORT_ID_INVALID;
>         } else {
>             RTE_ETH_FOREACH_MATCHING_DEV (port_id, devargs, &iterator) {
>                 /* If a break is done - must call rte_eth_iterator_cleanup. */
>                 rte_eth_iterator_cleanup(&iterator);
>                 break;
>             }
>         }
> 
> From my testing I see the call above to rte_dev_probe(devargs) flagging a
> DPDK error for attaching to a primary process (this is because
> 'local_dev_probe(') will return EEXIST for devices that were already probed in
> EAL init for OVS DPDK.). This does not occur in the hotplug case.
> 
> Although the error is flagged, DPDK then attempts to attach the port
> representor via secondary process with
> 'eal_dev_hotplug_request_to_secondary()'.
> 
> This seems to succeed as no error is returned here. However after returning
> from rte_dev_probe(devargs) (which returns 0), when we iterate over the ETH
> list again with
> 
> } else {
>             RTE_ETH_FOREACH_MATCHING_DEV (port_id, devargs, &iterator) {
>                 /* If a break is done - must call rte_eth_iterator_cleanup. */
>                 rte_eth_iterator_cleanup(&iterator);
>                 break;
>             }
> 
> It seems the secondary device (representor in this case) although added is not
> picked up in this iteration, so port_id remains as invalid.
> 
> As I said this only occurs when the PF has been probed on init (non hotplug 
> use
> case).
> 
> But even for the hotplug case it will also be a problem if you wish to add
> another representor that uses the same PF.
> 

When doing a hotplug with representors a driver must support multi probing (be 
able to be called by rte_dev_probe() on the same device several times in a raw 
and always returning successfully).
Prior to representors - DPDK drivers and ethdev library returned an error if a 
device was probed more than once.
Based on what you describe it seems that this functionality is missing in i40e 
and ixgbe.
Anyway, before testing this with OVS I would test it purely with DPDK by 
running testpmd and assigning the 2 representors you mention in a white list.
When testpmd is successful with representors - I would continue with OVS 
debugging.

In addition to that - in order to distinguish between PMDs supporting 
representors hotplug and those who do not - a driver should set flag 
RTE_ETH_DEV_CLOSE_REMOVE for representors.
OVS itself behaves differently depending on the existence of this flag. I 
notice this flag in several PMDs but not in i40e and ixgbe.
In addition a driver should implement the "remove()" callback in struct 
rte_pci_driver in order to automatically close all eth devices when it is 
unplugged.

> You may have resolved the issue in the v6 but I thought it was wise to flag 
> what
> the root cause of the problem I was hitting.
> 
> Ian
> >
> > Ian

Regards,
Ophir
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to