* [PATCH net-next RFC] virtio_net: ethtool tx napi configuration
@ 2018-09-12 23:29 Willem de Bruijn
2018-09-13 9:13 ` Jason Wang
2018-09-13 10:04 ` Tobin C. Harding
0 siblings, 2 replies; 4+ messages in thread
From: Willem de Bruijn @ 2018-09-12 23:29 UTC (permalink / raw)
To: netdev; +Cc: jasowang, mst, f.fainelli, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Implement ethtool .set_priv_flags and .get_priv_flags handlers
and use ethtool private flags to toggle transmit napi:
ethtool --set-priv-flags eth0 tx-napi on
ethtool --show-priv-flags eth0
Link: https://patchwork.ozlabs.org/patch/948149/
Suggested-by: Jason Wang <jasowang@redhat.com>
Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
drivers/net/virtio_net.c | 49 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 765920905226..9ca7e0a0f0d9 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -73,6 +73,10 @@ static const unsigned long guest_offloads[] = {
VIRTIO_NET_F_GUEST_UFO
};
+static const char virtnet_ethtool_priv_flags[][ETH_GSTRING_LEN] = {
+ "tx-napi",
+};
+
struct virtnet_stat_desc {
char desc[ETH_GSTRING_LEN];
size_t offset;
@@ -2059,6 +2063,9 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
}
}
break;
+ case ETH_SS_PRIV_FLAGS:
+ memcpy(data, virtnet_ethtool_priv_flags,
+ sizeof(virtnet_ethtool_priv_flags));
}
}
@@ -2070,6 +2077,9 @@ static int virtnet_get_sset_count(struct net_device *dev, int sset)
case ETH_SS_STATS:
return vi->curr_queue_pairs * (VIRTNET_RQ_STATS_LEN +
VIRTNET_SQ_STATS_LEN);
+ case ETH_SS_PRIV_FLAGS:
+ return ARRAY_SIZE(virtnet_ethtool_priv_flags);
+
default:
return -EOPNOTSUPP;
}
@@ -2181,6 +2191,43 @@ static int virtnet_get_link_ksettings(struct net_device *dev,
return 0;
}
+static int virtnet_set_priv_flags(struct net_device *dev, u32 priv_flags)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ int i, napi_weight;
+
+ napi_weight = priv_flags & 0x1 ? NAPI_POLL_WEIGHT : 0;
+
+ if (napi_weight ^ vi->sq[0].napi.weight) {
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ struct netdev_queue *txq =
+ netdev_get_tx_queue(vi->dev, i);
+
+ virtnet_napi_tx_disable(&vi->sq[i].napi);
+ __netif_tx_lock_bh(txq);
+ vi->sq[i].napi.weight = napi_weight;
+ if (!napi_weight)
+ virtqueue_enable_cb(vi->sq[i].vq);
+ __netif_tx_unlock_bh(txq);
+ virtnet_napi_tx_enable(vi, vi->sq[i].vq,
+ &vi->sq[i].napi);
+ }
+ }
+
+ return 0;
+}
+
+static u32 virtnet_get_priv_flags(struct net_device *dev)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ int priv_flags = 0;
+
+ if (vi->sq[0].napi.weight)
+ priv_flags |= 0x1;
+
+ return priv_flags;
+}
+
static void virtnet_init_settings(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -2219,6 +2266,8 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
.get_link_ksettings = virtnet_get_link_ksettings,
.set_link_ksettings = virtnet_set_link_ksettings,
+ .set_priv_flags = virtnet_set_priv_flags,
+ .get_priv_flags = virtnet_get_priv_flags,
};
static void virtnet_freeze_down(struct virtio_device *vdev)
--
2.19.0.rc2.392.g5ba43deb5a-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net-next RFC] virtio_net: ethtool tx napi configuration
2018-09-12 23:29 [PATCH net-next RFC] virtio_net: ethtool tx napi configuration Willem de Bruijn
@ 2018-09-13 9:13 ` Jason Wang
2018-09-13 10:04 ` Tobin C. Harding
1 sibling, 0 replies; 4+ messages in thread
From: Jason Wang @ 2018-09-13 9:13 UTC (permalink / raw)
To: Willem de Bruijn, netdev; +Cc: mst, f.fainelli, Willem de Bruijn
On 2018年09月13日 07:29, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Implement ethtool .set_priv_flags and .get_priv_flags handlers
> and use ethtool private flags to toggle transmit napi:
>
> ethtool --set-priv-flags eth0 tx-napi on
> ethtool --show-priv-flags eth0
>
> Link: https://patchwork.ozlabs.org/patch/948149/
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> drivers/net/virtio_net.c | 49 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 765920905226..9ca7e0a0f0d9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -73,6 +73,10 @@ static const unsigned long guest_offloads[] = {
> VIRTIO_NET_F_GUEST_UFO
> };
>
> +static const char virtnet_ethtool_priv_flags[][ETH_GSTRING_LEN] = {
> + "tx-napi",
> +};
> +
> struct virtnet_stat_desc {
> char desc[ETH_GSTRING_LEN];
> size_t offset;
> @@ -2059,6 +2063,9 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> }
> }
> break;
> + case ETH_SS_PRIV_FLAGS:
> + memcpy(data, virtnet_ethtool_priv_flags,
> + sizeof(virtnet_ethtool_priv_flags));
> }
> }
>
> @@ -2070,6 +2077,9 @@ static int virtnet_get_sset_count(struct net_device *dev, int sset)
> case ETH_SS_STATS:
> return vi->curr_queue_pairs * (VIRTNET_RQ_STATS_LEN +
> VIRTNET_SQ_STATS_LEN);
> + case ETH_SS_PRIV_FLAGS:
> + return ARRAY_SIZE(virtnet_ethtool_priv_flags);
> +
> default:
> return -EOPNOTSUPP;
> }
> @@ -2181,6 +2191,43 @@ static int virtnet_get_link_ksettings(struct net_device *dev,
> return 0;
> }
>
> +static int virtnet_set_priv_flags(struct net_device *dev, u32 priv_flags)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> + int i, napi_weight;
> +
> + napi_weight = priv_flags & 0x1 ? NAPI_POLL_WEIGHT : 0;
> +
> + if (napi_weight ^ vi->sq[0].napi.weight) {
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + struct netdev_queue *txq =
> + netdev_get_tx_queue(vi->dev, i);
> +
> + virtnet_napi_tx_disable(&vi->sq[i].napi);
> + __netif_tx_lock_bh(txq);
> + vi->sq[i].napi.weight = napi_weight;
> + if (!napi_weight)
> + virtqueue_enable_cb(vi->sq[i].vq);
I don't get why we need to disable enable cb here.
Thanks
> + __netif_tx_unlock_bh(txq);
> + virtnet_napi_tx_enable(vi, vi->sq[i].vq,
> + &vi->sq[i].napi);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static u32 virtnet_get_priv_flags(struct net_device *dev)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> + int priv_flags = 0;
> +
> + if (vi->sq[0].napi.weight)
> + priv_flags |= 0x1;
> +
> + return priv_flags;
> +}
> +
> static void virtnet_init_settings(struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> @@ -2219,6 +2266,8 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
> .get_ts_info = ethtool_op_get_ts_info,
> .get_link_ksettings = virtnet_get_link_ksettings,
> .set_link_ksettings = virtnet_set_link_ksettings,
> + .set_priv_flags = virtnet_set_priv_flags,
> + .get_priv_flags = virtnet_get_priv_flags,
> };
>
> static void virtnet_freeze_down(struct virtio_device *vdev)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net-next RFC] virtio_net: ethtool tx napi configuration
2018-09-12 23:29 [PATCH net-next RFC] virtio_net: ethtool tx napi configuration Willem de Bruijn
2018-09-13 9:13 ` Jason Wang
@ 2018-09-13 10:04 ` Tobin C. Harding
2018-09-13 15:00 ` Willem de Bruijn
1 sibling, 1 reply; 4+ messages in thread
From: Tobin C. Harding @ 2018-09-13 10:04 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, jasowang, mst, f.fainelli, Willem de Bruijn
On Wed, Sep 12, 2018 at 07:29:11PM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Implement ethtool .set_priv_flags and .get_priv_flags handlers
> and use ethtool private flags to toggle transmit napi:
>
> ethtool --set-priv-flags eth0 tx-napi on
> ethtool --show-priv-flags eth0
>
> Link: https://patchwork.ozlabs.org/patch/948149/
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> drivers/net/virtio_net.c | 49 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 765920905226..9ca7e0a0f0d9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -73,6 +73,10 @@ static const unsigned long guest_offloads[] = {
> VIRTIO_NET_F_GUEST_UFO
> };
>
> +static const char virtnet_ethtool_priv_flags[][ETH_GSTRING_LEN] = {
> + "tx-napi",
> +};
> +
> struct virtnet_stat_desc {
> char desc[ETH_GSTRING_LEN];
> size_t offset;
> @@ -2059,6 +2063,9 @@ static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> }
> }
> break;
> + case ETH_SS_PRIV_FLAGS:
> + memcpy(data, virtnet_ethtool_priv_flags,
> + sizeof(virtnet_ethtool_priv_flags));
> }
> }
>
> @@ -2070,6 +2077,9 @@ static int virtnet_get_sset_count(struct net_device *dev, int sset)
> case ETH_SS_STATS:
> return vi->curr_queue_pairs * (VIRTNET_RQ_STATS_LEN +
> VIRTNET_SQ_STATS_LEN);
> + case ETH_SS_PRIV_FLAGS:
> + return ARRAY_SIZE(virtnet_ethtool_priv_flags);
> +
> default:
> return -EOPNOTSUPP;
> }
> @@ -2181,6 +2191,43 @@ static int virtnet_get_link_ksettings(struct net_device *dev,
> return 0;
> }
>
> +static int virtnet_set_priv_flags(struct net_device *dev, u32 priv_flags)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> + int i, napi_weight;
> +
> + napi_weight = priv_flags & 0x1 ? NAPI_POLL_WEIGHT : 0;
> +
> + if (napi_weight ^ vi->sq[0].napi.weight) {
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + struct netdev_queue *txq =
> + netdev_get_tx_queue(vi->dev, i);
> +
> + virtnet_napi_tx_disable(&vi->sq[i].napi);
> + __netif_tx_lock_bh(txq);
> + vi->sq[i].napi.weight = napi_weight;
> + if (!napi_weight)
> + virtqueue_enable_cb(vi->sq[i].vq);
> + __netif_tx_unlock_bh(txq);
> + virtnet_napi_tx_enable(vi, vi->sq[i].vq,
> + &vi->sq[i].napi);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static u32 virtnet_get_priv_flags(struct net_device *dev)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> + int priv_flags = 0;
> +
> + if (vi->sq[0].napi.weight)
> + priv_flags |= 0x1;
> +
> + return priv_flags;
> +}
Why the use of priv_flags here? Is there some reason that we don't want
to use the more simple
static u32 virtnet_get_priv_flags(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
if (vi->sq[0].napi.weight)
return 1;
return 0;
}
thanks,
Tobin.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net-next RFC] virtio_net: ethtool tx napi configuration
2018-09-13 10:04 ` Tobin C. Harding
@ 2018-09-13 15:00 ` Willem de Bruijn
0 siblings, 0 replies; 4+ messages in thread
From: Willem de Bruijn @ 2018-09-13 15:00 UTC (permalink / raw)
To: me
Cc: Network Development, Jason Wang, Michael S. Tsirkin, f.fainelli,
Willem de Bruijn
> > +static u32 virtnet_get_priv_flags(struct net_device *dev)
> > +{
> > + struct virtnet_info *vi = netdev_priv(dev);
> > + int priv_flags = 0;
> > +
> > + if (vi->sq[0].napi.weight)
> > + priv_flags |= 0x1;
> > +
> > + return priv_flags;
> > +}
>
> Why the use of priv_flags here? Is there some reason that we don't want
> to use the more simple
>
> static u32 virtnet_get_priv_flags(struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
>
> if (vi->sq[0].napi.weight)
> return 1;
>
> return 0;
> }
Sure, that's fine, too.
I just wanted to make it explicit that this is one of possibly many
private flags,
and only acts on bit 0. If another private flag is added, the existing
code needs
little change, just add a branch on another bit. But either way works.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-13 20:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-12 23:29 [PATCH net-next RFC] virtio_net: ethtool tx napi configuration Willem de Bruijn
2018-09-13 9:13 ` Jason Wang
2018-09-13 10:04 ` Tobin C. Harding
2018-09-13 15:00 ` Willem de Bruijn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).