[v2,04/10] net/gve: add link update support
Checks
Commit Message
Support dev_ops link_update.
Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
---
drivers/net/gve/gve_ethdev.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
Comments
On 8/29/2022 9:41 AM, Junfeng Guo wrote:
>
> Support dev_ops link_update.
>
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> ---
> drivers/net/gve/gve_ethdev.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
> index f10f273f7d..435115c047 100644
> --- a/drivers/net/gve/gve_ethdev.c
> +++ b/drivers/net/gve/gve_ethdev.c
> @@ -37,10 +37,39 @@ gve_dev_configure(__rte_unused struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int
> +gve_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete)
> +{
> + struct gve_priv *priv = dev->data->dev_private;
> + struct rte_eth_link link;
> + int err;
> +
> + memset(&link, 0, sizeof(link));
> + link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
> + link.link_autoneg = RTE_ETH_LINK_AUTONEG;
> +
> + if (!dev->data->dev_started) {
> + link.link_status = RTE_ETH_LINK_DOWN;
> + link.link_speed = RTE_ETH_SPEED_NUM_NONE;
> + } else {
> + link.link_status = RTE_ETH_LINK_UP;
> + PMD_INIT_LOG(DEBUG, "Get link status from hw");
> + err = gve_adminq_report_link_speed(priv);
As far as I can see the API is calling an adminq command, is this
command blocking until link is up? If so is there a non blocking version
to utilize 'wait_to_complete', instead of ignoring it?
Also what will happen if 'start()' dev_ops called but cable is not
plugged in at all, won't this set link status still to "UP"?
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Friday, September 2, 2022 01:23
> To: Guo, Junfeng <junfeng.guo@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>;
> awogbemila@google.com; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: [PATCH v2 04/10] net/gve: add link update support
>
> On 8/29/2022 9:41 AM, Junfeng Guo wrote:
>
> >
> > Support dev_ops link_update.
> >
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
> > ---
> > drivers/net/gve/gve_ethdev.c | 30
> ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/drivers/net/gve/gve_ethdev.c
> b/drivers/net/gve/gve_ethdev.c
> > index f10f273f7d..435115c047 100644
> > --- a/drivers/net/gve/gve_ethdev.c
> > +++ b/drivers/net/gve/gve_ethdev.c
> > @@ -37,10 +37,39 @@ gve_dev_configure(__rte_unused struct
> rte_eth_dev *dev)
> > return 0;
> > }
> >
> > +static int
> > +gve_link_update(struct rte_eth_dev *dev, __rte_unused int
> wait_to_complete)
> > +{
> > + struct gve_priv *priv = dev->data->dev_private;
> > + struct rte_eth_link link;
> > + int err;
> > +
> > + memset(&link, 0, sizeof(link));
> > + link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
> > + link.link_autoneg = RTE_ETH_LINK_AUTONEG;
> > +
> > + if (!dev->data->dev_started) {
> > + link.link_status = RTE_ETH_LINK_DOWN;
> > + link.link_speed = RTE_ETH_SPEED_NUM_NONE;
> > + } else {
> > + link.link_status = RTE_ETH_LINK_UP;
> > + PMD_INIT_LOG(DEBUG, "Get link status from hw");
> > + err = gve_adminq_report_link_speed(priv);
>
> As far as I can see the API is calling an adminq command, is this
> command blocking until link is up? If so is there a non blocking version
> to utilize 'wait_to_complete', instead of ignoring it?
Yes, getting the link speed via an adminq command here is an blocking
until, and that's the only method we can utilize. As for the non-blocking
version, it depends on the real behavior of the HW (or backend), which
is like a black-box to us. There is no HW register to store the link status
info. Seems the 'wait_to_complete' cannot be used here. Thanks!
>
> Also what will happen if 'start()' dev_ops called but cable is not
> plugged in at all, won't this set link status still to "UP"?
Yes, that could be a terrible situation. Since this driver is running on the
cloud environment, we can only assume that the HW & backend part
work well when we run our driver. Thanks!
>
@@ -37,10 +37,39 @@ gve_dev_configure(__rte_unused struct rte_eth_dev *dev)
return 0;
}
+static int
+gve_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete)
+{
+ struct gve_priv *priv = dev->data->dev_private;
+ struct rte_eth_link link;
+ int err;
+
+ memset(&link, 0, sizeof(link));
+ link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
+ link.link_autoneg = RTE_ETH_LINK_AUTONEG;
+
+ if (!dev->data->dev_started) {
+ link.link_status = RTE_ETH_LINK_DOWN;
+ link.link_speed = RTE_ETH_SPEED_NUM_NONE;
+ } else {
+ link.link_status = RTE_ETH_LINK_UP;
+ PMD_INIT_LOG(DEBUG, "Get link status from hw");
+ err = gve_adminq_report_link_speed(priv);
+ if (err) {
+ PMD_DRV_LOG(ERR, "Failed to get link speed.");
+ priv->link_speed = RTE_ETH_SPEED_NUM_UNKNOWN;
+ }
+ link.link_speed = priv->link_speed;
+ }
+
+ return rte_eth_linkstatus_set(dev, &link);
+}
+
static int
gve_dev_start(struct rte_eth_dev *dev)
{
dev->data->dev_started = 1;
+ gve_link_update(dev, 0);
return 0;
}
@@ -73,6 +102,7 @@ static const struct eth_dev_ops gve_eth_dev_ops = {
.dev_start = gve_dev_start,
.dev_stop = gve_dev_stop,
.dev_close = gve_dev_close,
+ .link_update = gve_link_update,
};
static void