[dpdk-dev] [PATCH v1 1/2] net/ixgbevf: add an option pflink_fullchk to get link status quickly

[dpdk-dev] [PATCH v1 1/2] net/ixgbevf: add an option pflink_fullchk to get link status quickly

[Haiyue Wang]

In some scenario, if the PF's running status is stopped, the VF link
status can be detected as down by checking the mailbox status. This
fully checking will introduce mailbox interrupt in PF, and it will be
bad if many VFs are running and the application is checking the VF's
link status quickly (wait_to_complete is 0) and frequently.

Normally, checking the PF's link status is enough, no need to fully
check. For different scenarios, add 'pflink_fullchk' option to control
whether check the link fully or not.

Fixes: 91546fb62e67 ("net/ixgbevf: fix link state")
Cc: stable@dpdk.org

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 63 ++++++++++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++++
 2 files changed, 68 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 3636b50..ade119d 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -23,6 +23,7 @@
 #include <rte_bus_pci.h>
 #include <rte_branch_prediction.h>
 #include <rte_memory.h>
+#include <rte_kvargs.h>
 #include <rte_eal.h>
 #include <rte_alarm.h>
 #include <rte_ether.h>
@@ -127,6 +128,13 @@
 #define IXGBE_EXVET_VET_EXT_SHIFT              16
 #define IXGBE_DMATXCTL_VT_MASK                 0xFFFF0000
 
+#define IXGBEVF_DEVARG_PFLINK_FULLCHK		"pflink_fullchk"
+
+static const char * const ixgbevf_valid_arguments[] = {
+	IXGBEVF_DEVARG_PFLINK_FULLCHK,
+	NULL
+};
+
 static int eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params);
 static int eth_ixgbe_dev_uninit(struct rte_eth_dev *eth_dev);
 static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev);
@@ -1550,6 +1558,45 @@ generate_random_mac_addr(struct rte_ether_addr *mac_addr)
 	memcpy(&mac_addr->addr_bytes[3], &random, 3);
 }
 
+static int
+devarg_handle_int(__rte_unused const char *key, const char *value,
+		  void *extra_args)
+{
+	uint16_t *n = extra_args;
+
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
+	*n = (uint16_t)strtoul(value, NULL, 0);
+	if (*n == USHRT_MAX && errno == ERANGE)
+		return -1;
+
+	return 0;
+}
+
+static void
+ixgbevf_parse_devargs(struct ixgbe_adapter *adapter,
+		      struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist;
+	uint16_t pflink_fullchk;
+
+	if (devargs == NULL)
+		return;
+
+	kvlist = rte_kvargs_parse(devargs->args, ixgbevf_valid_arguments);
+	if (kvlist == NULL)
+		return;
+
+	if (rte_kvargs_count(kvlist, IXGBEVF_DEVARG_PFLINK_FULLCHK) == 1 &&
+	    rte_kvargs_process(kvlist, IXGBEVF_DEVARG_PFLINK_FULLCHK,
+			       devarg_handle_int, &pflink_fullchk) == 0 &&
+	    pflink_fullchk == 1)
+		adapter->pflink_fullchk = 1;
+
+	rte_kvargs_free(kvlist);
+}
+
 /*
  * Virtual Function device init
  */
@@ -1598,6 +1645,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
+	ixgbevf_parse_devargs(eth_dev->data->dev_private,
+			      pci_dev->device.devargs);
+
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
 	hw->device_id = pci_dev->id.device_id;
@@ -3910,6 +3960,8 @@ static int
 ixgbevf_check_link(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
 		   int *link_up, int wait_to_complete)
 {
+	struct ixgbe_adapter *adapter = container_of(hw,
+						     struct ixgbe_adapter, hw);
 	struct ixgbe_mbx_info *mbx = &hw->mbx;
 	struct ixgbe_mac_info *mac = &hw->mac;
 	uint32_t links_reg, in_msg;
@@ -3970,6 +4022,15 @@ ixgbevf_check_link(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
 		*speed = IXGBE_LINK_SPEED_UNKNOWN;
 	}
 
+	if (wait_to_complete == 0 && adapter->pflink_fullchk == 0) {
+		if (*speed == IXGBE_LINK_SPEED_UNKNOWN)
+			mac->get_link_status = true;
+		else
+			mac->get_link_status = false;
+
+		goto out;
+	}
+
 	/* if the read failed it could just be a mailbox collision, best wait
 	 * until we are called again and don't report an error
 	 */
@@ -8671,6 +8732,8 @@ RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic | vfio-pci");
 RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe_vf, "* igb_uio | vfio-pci");
+RTE_PMD_REGISTER_PARAM_STRING(net_ixgbe_vf,
+			      IXGBEVF_DEVARG_PFLINK_FULLCHK "=<0|1>");
 
 RTE_INIT(ixgbe_init_log)
 {
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index fdad94d..6e9ed2e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -498,6 +498,11 @@ struct ixgbe_adapter {
 
 	/* For RSS reta table update */
 	uint8_t rss_reta_updated;
+
+	/* Used for VF link sync with PF's physical and logical (by checking
+	 * mailbox status) link status.
+	 */
+	uint8_t pflink_fullchk;
 };
 
 struct ixgbe_vf_representor {
-- 
2.7.4

[dpdk-dev] [PATCH v1 2/2] doc: add the description for option pflink_fullchk in ixgbevf

[Haiyue Wang]

Add the detail description for this pflink_fullchk option, when it will
be used, and it is used for fixing what kind of issue.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 doc/guides/nics/ixgbe.rst | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index 975143f..33812a5 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -82,6 +82,24 @@ To guarantee the constraint, capabilities in dev_conf.rxmode.offloads will be ch
 
 fdir_conf->mode will also be checked.
 
+VF Runtime Options
+^^^^^^^^^^^^^^^^^^
+
+The following ``devargs`` options can be enabled at runtime. They must
+be passed as part of EAL arguments. For example,
+
+.. code-block:: console
+
+   testpmd -w af:10.0,pflink_fullchk=1 -- -i
+
+- ``pflink_fullchk`` (default **0**)
+
+  Toggle behavior to get ixgbevf link status quickly by checking the
+  mailbox status or not. If set, the VF will check the PF's link status
+  and the PF's mailbox running status, which will trigger PF's mailbox
+  interrupt generation. Otherwise, just check PF's link status, then no
+  mailbox interrupt in PF.
+
 RX Burst Size
 ^^^^^^^^^^^^^
 
-- 
2.7.4

[dpdk-dev] [PATCH 1/2] net/ixgbevf: add an option pflink_fullchk to get link status quickly

[Haiyue Wang]

In some scenario, if the PF's running status is stopped, the VF link
status can be detected as down by checking the mailbox status. This
fully checking will introduce mailbox interrupt in PF, and it will be
bad if many VFs are running and the application is checking the VF's
link status quickly (wait_to_complete is 0) and frequently.

Normally, checking the PF's link status is enough, no need to fully
check. For different scenarios, add 'pflink_fullchk' option to control
whether check the link fully or not.

Fixes: 91546fb62e67 ("net/ixgbevf: fix link state")
Cc: stable@dpdk.org

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 63 ++++++++++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++++
 2 files changed, 68 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 3636b50..ade119d 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -23,6 +23,7 @@
 #include <rte_bus_pci.h>
 #include <rte_branch_prediction.h>
 #include <rte_memory.h>
+#include <rte_kvargs.h>
 #include <rte_eal.h>
 #include <rte_alarm.h>
 #include <rte_ether.h>
@@ -127,6 +128,13 @@
 #define IXGBE_EXVET_VET_EXT_SHIFT              16
 #define IXGBE_DMATXCTL_VT_MASK                 0xFFFF0000
 
+#define IXGBEVF_DEVARG_PFLINK_FULLCHK		"pflink_fullchk"
+
+static const char * const ixgbevf_valid_arguments[] = {
+	IXGBEVF_DEVARG_PFLINK_FULLCHK,
+	NULL
+};
+
 static int eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params);
 static int eth_ixgbe_dev_uninit(struct rte_eth_dev *eth_dev);
 static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev);
@@ -1550,6 +1558,45 @@ generate_random_mac_addr(struct rte_ether_addr *mac_addr)
 	memcpy(&mac_addr->addr_bytes[3], &random, 3);
 }
 
+static int
+devarg_handle_int(__rte_unused const char *key, const char *value,
+		  void *extra_args)
+{
+	uint16_t *n = extra_args;
+
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
+	*n = (uint16_t)strtoul(value, NULL, 0);
+	if (*n == USHRT_MAX && errno == ERANGE)
+		return -1;
+
+	return 0;
+}
+
+static void
+ixgbevf_parse_devargs(struct ixgbe_adapter *adapter,
+		      struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist;
+	uint16_t pflink_fullchk;
+
+	if (devargs == NULL)
+		return;
+
+	kvlist = rte_kvargs_parse(devargs->args, ixgbevf_valid_arguments);
+	if (kvlist == NULL)
+		return;
+
+	if (rte_kvargs_count(kvlist, IXGBEVF_DEVARG_PFLINK_FULLCHK) == 1 &&
+	    rte_kvargs_process(kvlist, IXGBEVF_DEVARG_PFLINK_FULLCHK,
+			       devarg_handle_int, &pflink_fullchk) == 0 &&
+	    pflink_fullchk == 1)
+		adapter->pflink_fullchk = 1;
+
+	rte_kvargs_free(kvlist);
+}
+
 /*
  * Virtual Function device init
  */
@@ -1598,6 +1645,9 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 		return 0;
 	}
 
+	ixgbevf_parse_devargs(eth_dev->data->dev_private,
+			      pci_dev->device.devargs);
+
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
 	hw->device_id = pci_dev->id.device_id;
@@ -3910,6 +3960,8 @@ static int
 ixgbevf_check_link(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
 		   int *link_up, int wait_to_complete)
 {
+	struct ixgbe_adapter *adapter = container_of(hw,
+						     struct ixgbe_adapter, hw);
 	struct ixgbe_mbx_info *mbx = &hw->mbx;
 	struct ixgbe_mac_info *mac = &hw->mac;
 	uint32_t links_reg, in_msg;
@@ -3970,6 +4022,15 @@ ixgbevf_check_link(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
 		*speed = IXGBE_LINK_SPEED_UNKNOWN;
 	}
 
+	if (wait_to_complete == 0 && adapter->pflink_fullchk == 0) {
+		if (*speed == IXGBE_LINK_SPEED_UNKNOWN)
+			mac->get_link_status = true;
+		else
+			mac->get_link_status = false;
+
+		goto out;
+	}
+
 	/* if the read failed it could just be a mailbox collision, best wait
 	 * until we are called again and don't report an error
 	 */
@@ -8671,6 +8732,8 @@ RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic | vfio-pci");
 RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe_vf, "* igb_uio | vfio-pci");
+RTE_PMD_REGISTER_PARAM_STRING(net_ixgbe_vf,
+			      IXGBEVF_DEVARG_PFLINK_FULLCHK "=<0|1>");
 
 RTE_INIT(ixgbe_init_log)
 {
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index fdad94d..6e9ed2e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -498,6 +498,11 @@ struct ixgbe_adapter {
 
 	/* For RSS reta table update */
 	uint8_t rss_reta_updated;
+
+	/* Used for VF link sync with PF's physical and logical (by checking
+	 * mailbox status) link status.
+	 */
+	uint8_t pflink_fullchk;
 };
 
 struct ixgbe_vf_representor {
-- 
2.7.4

[dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf

[Haiyue Wang]

Add the detail description for this pflink_fullchk option, when it will
be used, and it is used for fixing what kind of issue.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 doc/guides/nics/ixgbe.rst | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index 975143f..33812a5 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -82,6 +82,24 @@ To guarantee the constraint, capabilities in dev_conf.rxmode.offloads will be ch
 
 fdir_conf->mode will also be checked.
 
+VF Runtime Options
+^^^^^^^^^^^^^^^^^^
+
+The following ``devargs`` options can be enabled at runtime. They must
+be passed as part of EAL arguments. For example,
+
+.. code-block:: console
+
+   testpmd -w af:10.0,pflink_fullchk=1 -- -i
+
+- ``pflink_fullchk`` (default **0**)
+
+  Toggle behavior to get ixgbevf link status quickly by checking the
+  mailbox status or not. If set, the VF will check the PF's link status
+  and the PF's mailbox running status, which will trigger PF's mailbox
+  interrupt generation. Otherwise, just check PF's link status, then no
+  mailbox interrupt in PF.
+
 RX Burst Size
 ^^^^^^^^^^^^^
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf

[Scott Daniels]

Acked-by: Scott Daniels daniels@research.att.com

On Thu, 6 Jun 2019, Haiyue Wang wrote:

> Add the detail description for this pflink_fullchk option, when it will
> be used, and it is used for fixing what kind of issue.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf

[Kevin Traynor]

On 07/06/2019 04:10, Haiyue Wang wrote:
> Add the detail description for this pflink_fullchk option, when it will
> be used, and it is used for fixing what kind of issue.
> 

You can squash the docs into the 1/2 patch.

> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
>  doc/guides/nics/ixgbe.rst | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> index 975143f..33812a5 100644
> --- a/doc/guides/nics/ixgbe.rst
> +++ b/doc/guides/nics/ixgbe.rst
> @@ -82,6 +82,24 @@ To guarantee the constraint, capabilities in dev_conf.rxmode.offloads will be ch
>  
>  fdir_conf->mode will also be checked.
>  
> +VF Runtime Options
> +^^^^^^^^^^^^^^^^^^
> +
> +The following ``devargs`` options can be enabled at runtime. They must
> +be passed as part of EAL arguments. For example,
> +
> +.. code-block:: console
> +
> +   testpmd -w af:10.0,pflink_fullchk=1 -- -i
> +
> +- ``pflink_fullchk`` (default **0**)
> +
> +  Toggle behavior to get ixgbevf link status quickly by checking the
> +  mailbox status or not. If set, the VF will check the PF's link status
> +  and the PF's mailbox running status, which will trigger PF's mailbox
> +  interrupt generation. Otherwise, just check PF's link status, then no
> +  mailbox interrupt in PF.
> +

wait_to_complete is also considered, so doesn't it mean that
pflink_fullck is not a toggle in all cases e.g. rte_eth_link_get() will
set wait_to_complete=1 and then pflink_fullchk is ignored. At least you
should mention if there is some user visible API like this where the
flag will not result in the behaviour being toggled.

>  RX Burst Size
>  ^^^^^^^^^^^^^
>  
> 

Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf

[Wang, Haiyue]

> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Friday, June 7, 2019 20:25
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Wang, Liang-min <liang-min.wang@intel.com>; daniels@research.att.com
> Subject: Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf
> 
> On 07/06/2019 04:10, Haiyue Wang wrote:
> > Add the detail description for this pflink_fullchk option, when it will
> > be used, and it is used for fixing what kind of issue.
> >
> 
> You can squash the docs into the 1/2 patch.
> 
Yes, I've sent a single patch before: https://patches.dpdk.org/patch/54374/
After some quiet time, I found that most of commits about doc are a single
patch, so I split it into two.

> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> >  doc/guides/nics/ixgbe.rst | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> > index 975143f..33812a5 100644
> > --- a/doc/guides/nics/ixgbe.rst
> > +++ b/doc/guides/nics/ixgbe.rst
> > @@ -82,6 +82,24 @@ To guarantee the constraint, capabilities in dev_conf.rxmode.offloads will be ch
> >
> >  fdir_conf->mode will also be checked.
> >
> > +VF Runtime Options
> > +^^^^^^^^^^^^^^^^^^
> > +
> > +The following ``devargs`` options can be enabled at runtime. They must
> > +be passed as part of EAL arguments. For example,
> > +
> > +.. code-block:: console
> > +
> > +   testpmd -w af:10.0,pflink_fullchk=1 -- -i
> > +
> > +- ``pflink_fullchk`` (default **0**)
> > +
> > +  Toggle behavior to get ixgbevf link status quickly by checking the
> > +  mailbox status or not. If set, the VF will check the PF's link status
> > +  and the PF's mailbox running status, which will trigger PF's mailbox
> > +  interrupt generation. Otherwise, just check PF's link status, then no
> > +  mailbox interrupt in PF.
> > +
> 
> wait_to_complete is also considered, so doesn't it mean that
> pflink_fullck is not a toggle in all cases e.g. rte_eth_link_get() will
> set wait_to_complete=1 and then pflink_fullchk is ignored. At least you
> should mention if there is some user visible API like this where the
> flag will not result in the behaviour being toggled.
> 

I use "get ixgbevf link status -quickly-" to describe "wait_to_complete = 0",
maybe I need some code to make the information more clear.

> >  RX Burst Size
> >  ^^^^^^^^^^^^^
> >
> >

Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf

[Kevin Traynor]

On 07/06/2019 14:55, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Kevin Traynor [mailto:ktraynor@redhat.com]
>> Sent: Friday, June 7, 2019 20:25
>> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu,
>> Wenzhuo <wenzhuo.lu@intel.com>; Wang, Liang-min <liang-min.wang@intel.com>; daniels@research.att.com
>> Subject: Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf
>>
>> On 07/06/2019 04:10, Haiyue Wang wrote:
>>> Add the detail description for this pflink_fullchk option, when it will
>>> be used, and it is used for fixing what kind of issue.
>>>
>>
>> You can squash the docs into the 1/2 patch.
>>
> Yes, I've sent a single patch before: https://patches.dpdk.org/patch/54374/
> After some quiet time, I found that most of commits about doc are a single
> patch, so I split it into two.
> 

Personally I don't have an issue either way, but docs with code is the
preferred method for DPDK so you could squash if you are doing a v2.

>>> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>>> ---
>>>  doc/guides/nics/ixgbe.rst | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
>>> index 975143f..33812a5 100644
>>> --- a/doc/guides/nics/ixgbe.rst
>>> +++ b/doc/guides/nics/ixgbe.rst
>>> @@ -82,6 +82,24 @@ To guarantee the constraint, capabilities in dev_conf.rxmode.offloads will be ch
>>>
>>>  fdir_conf->mode will also be checked.
>>>
>>> +VF Runtime Options
>>> +^^^^^^^^^^^^^^^^^^
>>> +
>>> +The following ``devargs`` options can be enabled at runtime. They must
>>> +be passed as part of EAL arguments. For example,
>>> +
>>> +.. code-block:: console
>>> +
>>> +   testpmd -w af:10.0,pflink_fullchk=1 -- -i
>>> +
>>> +- ``pflink_fullchk`` (default **0**)
>>> +
>>> +  Toggle behavior to get ixgbevf link status quickly by checking the
>>> +  mailbox status or not. If set, the VF will check the PF's link status
>>> +  and the PF's mailbox running status, which will trigger PF's mailbox
>>> +  interrupt generation. Otherwise, just check PF's link status, then no
>>> +  mailbox interrupt in PF.
>>> +
>>
>> wait_to_complete is also considered, so doesn't it mean that
>> pflink_fullck is not a toggle in all cases e.g. rte_eth_link_get() will
>> set wait_to_complete=1 and then pflink_fullchk is ignored. At least you
>> should mention if there is some user visible API like this where the
>> flag will not result in the behaviour being toggled.
>>
> 
> I use "get ixgbevf link status -quickly-" to describe "wait_to_complete = 0",
> maybe I need some code to make the information more clear.
> 

I think all you need to add is that rte_eth_link_get() will still use
the mailbox method regardless of the pflink_fullchk setting, but maybe
there's more I haven't considered wrt testpmd.

>>>  RX Burst Size
>>>  ^^^^^^^^^^^^^
>>>
>>>
> 

Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf

[Wang, Haiyue]

> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Friday, June 7, 2019 22:38
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Wang, Liang-min <liang-min.wang@intel.com>; daniels@research.att.com
> Subject: Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf
> 
> On 07/06/2019 14:55, Wang, Haiyue wrote:
> >> -----Original Message-----
> >> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> >> Sent: Friday, June 7, 2019 20:25
> >> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu,
> >> Wenzhuo <wenzhuo.lu@intel.com>; Wang, Liang-min <liang-min.wang@intel.com>;
> daniels@research.att.com
> >> Subject: Re: [dpdk-dev] [PATCH 2/2] doc: add the description for option pflink_fullchk in ixgbevf
> >>
> >> On 07/06/2019 04:10, Haiyue Wang wrote:
> >>> Add the detail description for this pflink_fullchk option, when it will
> >>> be used, and it is used for fixing what kind of issue.
> >>>
> >>
> >> You can squash the docs into the 1/2 patch.
> >>
> > Yes, I've sent a single patch before: https://patches.dpdk.org/patch/54374/
> > After some quiet time, I found that most of commits about doc are a single
> > patch, so I split it into two.
> >
> 
> Personally I don't have an issue either way, but docs with code is the
> preferred method for DPDK so you could squash if you are doing a v2.
> 

One patch sounds better now, thanks!

> >>> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >>> ---
> >>>  doc/guides/nics/ixgbe.rst | 18 ++++++++++++++++++
> >>>  1 file changed, 18 insertions(+)
> >>>
> >>> diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> >>> index 975143f..33812a5 100644
> >>> --- a/doc/guides/nics/ixgbe.rst
> >>> +++ b/doc/guides/nics/ixgbe.rst
> >>> @@ -82,6 +82,24 @@ To guarantee the constraint, capabilities in dev_conf.rxmode.offloads will be
> ch
> >>>
> >>>  fdir_conf->mode will also be checked.
> >>>
> >>> +VF Runtime Options
> >>> +^^^^^^^^^^^^^^^^^^
> >>> +
> >>> +The following ``devargs`` options can be enabled at runtime. They must
> >>> +be passed as part of EAL arguments. For example,
> >>> +
> >>> +.. code-block:: console
> >>> +
> >>> +   testpmd -w af:10.0,pflink_fullchk=1 -- -i
> >>> +
> >>> +- ``pflink_fullchk`` (default **0**)
> >>> +
> >>> +  Toggle behavior to get ixgbevf link status quickly by checking the
> >>> +  mailbox status or not. If set, the VF will check the PF's link status
> >>> +  and the PF's mailbox running status, which will trigger PF's mailbox
> >>> +  interrupt generation. Otherwise, just check PF's link status, then no
> >>> +  mailbox interrupt in PF.
> >>> +
> >>
> >> wait_to_complete is also considered, so doesn't it mean that
> >> pflink_fullck is not a toggle in all cases e.g. rte_eth_link_get() will
> >> set wait_to_complete=1 and then pflink_fullchk is ignored. At least you
> >> should mention if there is some user visible API like this where the
> >> flag will not result in the behaviour being toggled.
> >>
> >
> > I use "get ixgbevf link status -quickly-" to describe "wait_to_complete = 0",
> > maybe I need some code to make the information more clear.
> >
> 
> I think all you need to add is that rte_eth_link_get() will still use
> the mailbox method regardless of the pflink_fullchk setting, but maybe
> there's more I haven't considered wrt testpmd.
> 

Agree, will update a new patch to make this clear. Thanks!

> >>>  RX Burst Size
> >>>  ^^^^^^^^^^^^^
> >>>
> >>>
> >