| Message ID | 20241218144530.2963326-10-ap420073@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bnxt_en: implement tcp-data-split and thresh option | expand |
On Wed, 18 Dec 2024 14:45:30 +0000 Taehee Yoo wrote: > drivers/net/netdevsim/ethtool.c | 15 ++++++++++++++- > drivers/net/netdevsim/netdevsim.h | 4 ++++ netdevsim patches must come with a selftest that uses them :) We support bash, C and Python tests. Look inside tools/testing/selftests/drivers/net/ These tests could serve as examples: stats.py, hw/ncdevmem.c TBH it doesn't look like netdevsim _actually_ supports HDS, as in forwarded packets will not be split with the current code, or linearized. You'd need to modify nsim_start_xmit() to either linearize the packet or not based on *peer's* HDS settings.
On Thu, Dec 19, 2024 at 11:49 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 18 Dec 2024 14:45:30 +0000 Taehee Yoo wrote: > > drivers/net/netdevsim/ethtool.c | 15 ++++++++++++++- > > drivers/net/netdevsim/netdevsim.h | 4 ++++ > > netdevsim patches must come with a selftest that uses them :) > We support bash, C and Python tests. Look inside > tools/testing/selftests/drivers/net/ > These tests could serve as examples: stats.py, hw/ncdevmem.c > > TBH it doesn't look like netdevsim _actually_ supports HDS, > as in forwarded packets will not be split with the current > code, or linearized. You'd need to modify nsim_start_xmit() > to either linearize the packet or not based on *peer's* HDS > settings. Ah, Sorry for the sloppy change. I will write a selftest. Also, I will modify nsim_start_xmit() to support HDS. The example would be very helpful to me. Thank you so much! Taehee Yoo.
On Thu, 19 Dec 2024 23:37:45 +0900 Taehee Yoo wrote:
> The example would be very helpful to me.
Just to make sure nothing gets lost in translation, are you saying that:
- the examples of tests I listed are useful; or
- you'd appreciate examples of how to code up HDS in netdevsim; or
- you'd appreciate more suitable examples of the tests?
:)
On Thu, Dec 19, 2024 at 11:45 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 19 Dec 2024 23:37:45 +0900 Taehee Yoo wrote: > > The example would be very helpful to me. > > Just to make sure nothing gets lost in translation, are you saying that: > - the examples of tests I listed are useful; or > - you'd appreciate examples of how to code up HDS in netdevsim; or > - you'd appreciate more suitable examples of the tests? > > :) Ah, I appreciate example of tests you listed are useful, Thanks!
On Fri, 20 Dec 2024 00:19:38 +0900 Taehee Yoo wrote: > > On Thu, 19 Dec 2024 23:37:45 +0900 Taehee Yoo wrote: > > > The example would be very helpful to me. > > > > Just to make sure nothing gets lost in translation, are you saying that: > > - the examples of tests I listed are useful; or > > - you'd appreciate examples of how to code up HDS in netdevsim; or > > - you'd appreciate more suitable examples of the tests? > > > > :) > > Ah, I appreciate example of tests you listed are useful, Thanks! Okay :) FWIW I don't expect that you'd do anything too complicated to support HDS in netdevsim. The packets generated by the networking stack are "split", already. What I was thinking is basically check if HDS is enabled, compare skb->len to threshold, and if we shouldn't HDS call skb_linearize().
diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c index 5fe1eaef99b5..aa176f52fc3f 100644 --- a/drivers/net/netdevsim/ethtool.c +++ b/drivers/net/netdevsim/ethtool.c @@ -2,7 +2,6 @@ // Copyright (c) 2020 Facebook #include <linux/debugfs.h> -#include <linux/ethtool.h> #include <linux/random.h> #include "netdevsim.h" @@ -72,6 +71,11 @@ static void nsim_get_ringparam(struct net_device *dev, struct netdevsim *ns = netdev_priv(dev); memcpy(ring, &ns->ethtool.ring, sizeof(ns->ethtool.ring)); + memcpy(kernel_ring, &ns->ethtool.kernel_ring, + sizeof(ns->ethtool.kernel_ring)); + + if (kernel_ring->tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_UNKNOWN) + kernel_ring->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_DISABLED; } static int nsim_set_ringparam(struct net_device *dev, @@ -85,6 +89,9 @@ static int nsim_set_ringparam(struct net_device *dev, ns->ethtool.ring.rx_jumbo_pending = ring->rx_jumbo_pending; ns->ethtool.ring.rx_mini_pending = ring->rx_mini_pending; ns->ethtool.ring.tx_pending = ring->tx_pending; + ns->ethtool.kernel_ring.tcp_data_split = kernel_ring->tcp_data_split; + ns->ethtool.kernel_ring.hds_thresh = kernel_ring->hds_thresh; + return 0; } @@ -161,6 +168,8 @@ static int nsim_get_ts_info(struct net_device *dev, static const struct ethtool_ops nsim_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_ALL_PARAMS, + .supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT | + ETHTOOL_RING_USE_HDS_THRS, .get_pause_stats = nsim_get_pause_stats, .get_pauseparam = nsim_get_pauseparam, .set_pauseparam = nsim_set_pauseparam, @@ -182,6 +191,10 @@ static void nsim_ethtool_ring_init(struct netdevsim *ns) ns->ethtool.ring.rx_jumbo_max_pending = 4096; ns->ethtool.ring.rx_mini_max_pending = 4096; ns->ethtool.ring.tx_max_pending = 4096; + + ns->ethtool.kernel_ring.tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_UNKNOWN; + ns->ethtool.kernel_ring.hds_thresh = 0; + ns->ethtool.kernel_ring.hds_thresh_max = NSIM_HDS_THRESHOLD_MAX; } void nsim_ethtool_init(struct netdevsim *ns) diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h index bf02efa10956..6abbc627308d 100644 --- a/drivers/net/netdevsim/netdevsim.h +++ b/drivers/net/netdevsim/netdevsim.h @@ -16,6 +16,7 @@ #include <linux/debugfs.h> #include <linux/device.h> #include <linux/ethtool.h> +#include <linux/ethtool_netlink.h> #include <linux/kernel.h> #include <linux/list.h> #include <linux/netdevice.h> @@ -36,6 +37,8 @@ #define NSIM_IPSEC_VALID BIT(31) #define NSIM_UDP_TUNNEL_N_PORTS 4 +#define NSIM_HDS_THRESHOLD_MAX 1024 + struct nsim_sa { struct xfrm_state *xs; __be32 ipaddr[4]; @@ -87,6 +90,7 @@ struct nsim_ethtool { struct nsim_ethtool_pauseparam pauseparam; struct ethtool_coalesce coalesce; struct ethtool_ringparam ring; + struct kernel_ethtool_ringparam kernel_ring; struct ethtool_fecparam fec; };
HDS options(tcp-data-split, hds-thresh) have dendencies between other features like XDP. Basic dependencies are checked in the core API. netdevsim is very useful to check basic dependencies. The default tcp-data-split mode is UNKNOWN but netdevsim driver returns DISABLED when ethtool dumps tcp-data-split mode. The default value of HDS threshold is 0 and the maximum value is 1024. ethtool shows like this. ethtool -g eni1np1 Ring parameters for eni1np1: Pre-set maximums: ... HDS thresh: 1024 Current hardware settings: ... TCP data split: off HDS thresh: 0 ethtool -G eni1np1 tcp-data-split on hds-thresh 1024 ethtool -g eni1np1 Ring parameters for eni1np1: Pre-set maximums: ... HDS thresh: 1024 Current hardware settings: ... TCP data split: on HDS thresh: 1024 Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- v6: - Patch added. drivers/net/netdevsim/ethtool.c | 15 ++++++++++++++- drivers/net/netdevsim/netdevsim.h | 4 ++++ 2 files changed, 18 insertions(+), 1 deletion(-)