Re: [dpdk-dev] [PATCH v4 04/12] net/failsafe: add fail-safe PMD

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

On Wed, May 31, 2017 at 08:13:53AM -0700, Stephen Hemminger wrote:
> On Mon, 29 May 2017 15:42:16 +0200
> Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > +Fail-safe poll mode driver library
> > +==================================
> > +
> > +The Fail-safe poll mode driver library (**librte_pmd_failsafe**) is a virtual
> > +device that allows using any device supporting hotplug (sudden device removal
> > +and plugging on its bus), without modifying other components relying on such
> > +device (application, other PMDs).
> 
> What about the case of Hyper-V where the components of the Fail Safe PMD may
> arrive later. An example would be a NFV server that starts on boot. The synthetic
> device will be present at boot, but the associated VF device may be plugged
> in later (by checking SR-IOV on host console) or removed (by unchecking).
> 
> There doesn't appear to be a way to manage slave devices that get added
> and removed through CLI management model.
> 
> 
> 

The VF and the synthetic path (SP) should both be declared as slaves to the
fail-safe. The SP is probed while the process fails for the VF.

The fail-safe then continues as usual, getting his infos (MAC address,
capabilities) from the SP. More on that later, as you have evocated the
subject in another thread.

The fail-safe detects that not all his slaves are probed and enables its
plugin poll, meaning that it will detect when the VF arrives.

As the VF appears later, there is no way to know which PCI address it
will be at. Thus the need for the "exec" slave declaration, which allows
complex logic for slave detection.

What is necessary is a common piece of info (it can be MAC address, a
class Id, anything else) that allows a script to detect that the right
device has been plugged in. As long as the NFV server allows determinism
here, the user will be able to use its VF.

> > +Using the Fail-safe PMD from the EAL command line
> > +-------------------------------------------------
> > +
> > +The Fail-safe PMD can be used like most other DPDK virtual devices, by passing a
> > +``--vdev`` parameter to the EAL when starting the application. The device name
> > +must start with the *net_failsafe* prefix, followed by numbers or letters. This
> > +name must be unique for each device. Each fail-safe instance must have at least one
> > +sub-device, up to ``RTE_MAX_ETHPORTS-1``.
> > +
> > +A sub-device can be any legal DPDK device, including possibly another fail-safe
> > +instance.
> 
> Configuring fail-safe (or any other device) from command line is difficult in a real
> world application. The EAL command line is difficult API to manipulate programmatically.
> Why not have a real API?
> 

The real API is proposed through the standard DPDK layers.
You can already create a virtual device on the fly with arbitrary
parameters. You can thus create a fail-safe device with several slaves.

The requirement to be able to do this however is that the bus of the
slave supports the plug / unplug API. This is the case for the virtual
and PCI buses.

You can try it on testpmd, using a command such as

testpmd> port attach net_failsafe0,dev(net_ring0),dev(net_ring1)

Should create a fail-safe instance with two slaves.

Finally, in a recent patchset, I introduced an rte_devargs parsing
helper that should ease the creation of devices in this way. It takes a
"name,devargs" string and builds an rte_devargs, that can be used in any
plug/unplug implementation worth its salt.

{
  struct rte_devargs da;

  rte_eal_devargs_parse("net_failsafe0,dev(net_ring0)", &da);
  da.bus->plug(&da);
}

And you are set.

> > +static int
> > +fs_link_update(struct rte_eth_dev *dev,
> > +		int wait_to_complete)
> > +{
> > +	struct sub_device *sdev;
> > +	uint8_t i;
> > +	int ret;
> > +
> > +	FOREACH_SUBDEV_ST(sdev, i, dev, DEV_ACTIVE) {
> > +		DEBUG("Calling link_update on sub_device %d", i);
> > +		ret = (SUBOPS(sdev, link_update))(ETH(sdev), wait_to_complete);
> > +		if (ret && ret != -1) {
> > +			ERROR("Link update failed for sub_device %d with error %d",
> > +			      i, ret);
> > +			return ret;
> > +		}
> > +	}
> > +	if (TX_SUBDEV(dev)) {
> > +		struct rte_eth_link *l1;
> > +		struct rte_eth_link *l2;
> > +
> > +		l1 = &dev->data->dev_link;
> > +		l2 = &ETH(TX_SUBDEV(dev))->data->dev_link;
> > +		if (memcmp(l1, l2, sizeof(*l1))) {
> > +			*l1 = *l2;
> > +			return 0;
> > +		}
> > +	}
> > +	return -1;
> > +}
> 
> memcmp here is a potential problem since rte_eth_link maybe padded and have holes.
> Why compare anyway? if *l1 == *l2 the assignment would be a nop.
> What if links are down?
> 
> 
> > +static void
> > +fs_stats_get(struct rte_eth_dev *dev,
> > +	     struct rte_eth_stats *stats)
> > +{
> > +	memset(stats, 0, sizeof(*stats));
> 
> memset here is unnecessary, already done by rte_eth_stats_get

-- 
Gaëtan Rivet
6WIND