[bpf-next,3/6] selftests/xsk: implement get_hw_ring_size function to retrieve current and max interface size

Commit Message

Tushar Vyavahare March 15, 2024, 2:07 p.m. UTC

Introduce a new function called get_hw_size that retrieves both the
current and maximum size of the interface and stores this information in
the 'hw_ring' structure.

Signed-off-by: Tushar Vyavahare <[e-mail]>
---
 tools/testing/selftests/bpf/xskxceiver.c | 32 ++++++++++++++++++++++++
 tools/testing/selftests/bpf/xskxceiver.h |  8 ++++++
 2 files changed, 40 insertions(+)

Comments

Stanislav Fomichev March 15, 2024, 5:40 p.m. UTC | #1

On 03/15, Tushar Vyavahare wrote:
> Introduce a new function called get_hw_size that retrieves both the
> current and maximum size of the interface and stores this information in
> the 'hw_ring' structure.
> 
> Signed-off-by: Tushar Vyavahare <[e-mail]>
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 32 ++++++++++++++++++++++++
>  tools/testing/selftests/bpf/xskxceiver.h |  8 ++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index eaa102c8098b..32005bfb9c9f 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -81,6 +81,8 @@
>  #include <linux/mman.h>
>  #include <linux/netdev.h>
>  #include <linux/bitmap.h>
> +#include <linux/sockios.h>
> +#include <linux/ethtool.h>
>  #include <arpa/inet.h>
>  #include <net/if.h>
>  #include <locale.h>
> @@ -95,6 +97,7 @@
>  #include <sys/socket.h>
>  #include <sys/time.h>
>  #include <sys/types.h>
> +#include <sys/ioctl.h>
>  #include <unistd.h>
>  
>  #include "xsk_xdp_progs.skel.h"
> @@ -409,6 +412,35 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  	}
>  }
>  
> +static int get_hw_ring_size(struct ifobject *ifobj)
> +{
> +	struct ethtool_ringparam ring_param = {0};
> +	struct ifreq ifr = {0};
> +	int sockfd;
> +
> +	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> +	if (sockfd < 0)
> +		return errno;
> +
> +	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> +
> +	ring_param.cmd = ETHTOOL_GRINGPARAM;
> +	ifr.ifr_data = (char *)&ring_param;
> +
> +	if (ioctl(sockfd, SIOCETHTOOL, &ifr) < 0) {
> +		close(sockfd);
> +		return errno;

close(sockfd) can potentially override the errno. Also, return -errno to
match the other cases where errors are < 0.

> +	}
> +
> +	ifobj->ring.default_tx = ring_param.tx_pending;
> +	ifobj->ring.default_rx = ring_param.rx_pending;
> +	ifobj->ring.max_tx = ring_param.tx_max_pending;
> +	ifobj->ring.max_rx = ring_param.rx_max_pending;
> +
> +	close(sockfd);
> +	return 0;
> +}
> +
>  static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  			     struct ifobject *ifobj_rx)
>  {
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index 425304e52f35..4f58b70fa781 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -114,6 +114,13 @@ struct pkt_stream {
>  	bool verbatim;
>  };
>  
> +struct hw_ring {
> +	u32 default_tx;
> +	u32 default_rx;
> +	u32 max_tx;
> +	u32 max_rx;
> +};
> +
>  struct ifobject;
>  struct test_spec;
>  typedef int (*validation_func_t)(struct ifobject *ifobj);
> @@ -130,6 +137,7 @@ struct ifobject {
>  	struct xsk_xdp_progs *xdp_progs;
>  	struct bpf_map *xskmap;
>  	struct bpf_program *xdp_prog;
> +	struct hw_ring ring;

Any reason not to store ethtool_ringparam directly here? No need to
introduce new hw_ring.

Tushar Vyavahare March 19, 2024, 6:47 a.m. UTC | #2

> -----Original Message-----
> From: Stanislav Fomichev <[e-mail]>
> Sent: Friday, March 15, 2024 11:11 PM
> To: Vyavahare, Tushar <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail]; Karlsson,
> Magnus <[e-mail]>; Fijalkowski, Maciej
> <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Sarkar, Tirthendu
> <[e-mail]>
> Subject: Re: [PATCH bpf-next 3/6] selftests/xsk: implement get_hw_ring_size
> function to retrieve current and max interface size
> 
> On 03/15, Tushar Vyavahare wrote:
> > Introduce a new function called get_hw_size that retrieves both the
> > current and maximum size of the interface and stores this information
> > in the 'hw_ring' structure.
> >
> > Signed-off-by: Tushar Vyavahare <[e-mail]>
> > ---
> >  tools/testing/selftests/bpf/xskxceiver.c | 32
> > ++++++++++++++++++++++++  tools/testing/selftests/bpf/xskxceiver.h |
> > 8 ++++++
> >  2 files changed, 40 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c
> > b/tools/testing/selftests/bpf/xskxceiver.c
> > index eaa102c8098b..32005bfb9c9f 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -81,6 +81,8 @@
> >  #include <linux/mman.h>
> >  #include <linux/netdev.h>
> >  #include <linux/bitmap.h>
> > +#include <linux/sockios.h>
> > +#include <linux/ethtool.h>
> >  #include <arpa/inet.h>
> >  #include <net/if.h>
> >  #include <locale.h>
> > @@ -95,6 +97,7 @@
> >  #include <sys/socket.h>
> >  #include <sys/time.h>
> >  #include <sys/types.h>
> > +#include <sys/ioctl.h>
> >  #include <unistd.h>
> >
> >  #include "xsk_xdp_progs.skel.h"
> > @@ -409,6 +412,35 @@ static void parse_command_line(struct ifobject
> *ifobj_tx, struct ifobject *ifobj
> >  	}
> >  }
> >
> > +static int get_hw_ring_size(struct ifobject *ifobj) {
> > +	struct ethtool_ringparam ring_param = {0};
> > +	struct ifreq ifr = {0};
> > +	int sockfd;
> > +
> > +	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
> > +	if (sockfd < 0)
> > +		return errno;
> > +
> > +	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
> > +
> > +	ring_param.cmd = ETHTOOL_GRINGPARAM;
> > +	ifr.ifr_data = (char *)&ring_param;
> > +
> > +	if (ioctl(sockfd, SIOCETHTOOL, &ifr) < 0) {
> > +		close(sockfd);
> > +		return errno;
> 
> close(sockfd) can potentially override the errno. Also, return -errno to match
> the other cases where errors are < 0.
> 

I will do it.

> > +	}
> > +
> > +	ifobj->ring.default_tx = ring_param.tx_pending;
> > +	ifobj->ring.default_rx = ring_param.rx_pending;
> > +	ifobj->ring.max_tx = ring_param.tx_max_pending;
> > +	ifobj->ring.max_rx = ring_param.rx_max_pending;
> > +
> > +	close(sockfd);
> > +	return 0;
> > +}
> > +
> >  static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >  			     struct ifobject *ifobj_rx)
> >  {
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h
> > b/tools/testing/selftests/bpf/xskxceiver.h
> > index 425304e52f35..4f58b70fa781 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -114,6 +114,13 @@ struct pkt_stream {
> >  	bool verbatim;
> >  };
> >
> > +struct hw_ring {
> > +	u32 default_tx;
> > +	u32 default_rx;
> > +	u32 max_tx;
> > +	u32 max_rx;
> > +};
> > +
> >  struct ifobject;
> >  struct test_spec;
> >  typedef int (*validation_func_t)(struct ifobject *ifobj); @@ -130,6
> > +137,7 @@ struct ifobject {
> >  	struct xsk_xdp_progs *xdp_progs;
> >  	struct bpf_map *xskmap;
> >  	struct bpf_program *xdp_prog;
> > +	struct hw_ring ring;
> 
> Any reason not to store ethtool_ringparam directly here? No need to
> introduce new hw_ring.

I will use ethtool_ringparam directly for get_hw_ring_size().

Patch

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index eaa102c8098b..32005bfb9c9f 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -81,6 +81,8 @@ 
 #include <linux/mman.h>
 #include <linux/netdev.h>
 #include <linux/bitmap.h>
+#include <linux/sockios.h>
+#include <linux/ethtool.h>
 #include <arpa/inet.h>
 #include <net/if.h>
 #include <locale.h>
@@ -95,6 +97,7 @@ 
 #include <sys/socket.h>
 #include <sys/time.h>
 #include <sys/types.h>
+#include <sys/ioctl.h>
 #include <unistd.h>
 
 #include "xsk_xdp_progs.skel.h"
@@ -409,6 +412,35 @@  static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 	}
 }
 
+static int get_hw_ring_size(struct ifobject *ifobj)
+{
+	struct ethtool_ringparam ring_param = {0};
+	struct ifreq ifr = {0};
+	int sockfd;
+
+	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
+	if (sockfd < 0)
+		return errno;
+
+	memcpy(ifr.ifr_name, ifobj->ifname, sizeof(ifr.ifr_name));
+
+	ring_param.cmd = ETHTOOL_GRINGPARAM;
+	ifr.ifr_data = (char *)&ring_param;
+
+	if (ioctl(sockfd, SIOCETHTOOL, &ifr) < 0) {
+		close(sockfd);
+		return errno;
+	}
+
+	ifobj->ring.default_tx = ring_param.tx_pending;
+	ifobj->ring.default_rx = ring_param.rx_pending;
+	ifobj->ring.max_tx = ring_param.tx_max_pending;
+	ifobj->ring.max_rx = ring_param.rx_max_pending;
+
+	close(sockfd);
+	return 0;
+}
+
 static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			     struct ifobject *ifobj_rx)
 {
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index 425304e52f35..4f58b70fa781 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -114,6 +114,13 @@  struct pkt_stream {
 	bool verbatim;
 };
 
+struct hw_ring {
+	u32 default_tx;
+	u32 default_rx;
+	u32 max_tx;
+	u32 max_rx;
+};
+
 struct ifobject;
 struct test_spec;
 typedef int (*validation_func_t)(struct ifobject *ifobj);
@@ -130,6 +137,7 @@  struct ifobject {
 	struct xsk_xdp_progs *xdp_progs;
 	struct bpf_map *xskmap;
 	struct bpf_program *xdp_prog;
+	struct hw_ring ring;
 	enum test_mode mode;
 	int ifindex;
 	int mtu;