> -----Original Message-----
> From: Stanislav Fomichev <sdf@xxxxxxxxxx>
> Sent: Friday, March 15, 2024 11:11 PM
> To: Vyavahare, Tushar <tushar.vyavahare@xxxxxxxxx>
> Cc: bpf@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; bjorn@xxxxxxxxxx; Karlsson,
> Magnus <magnus.karlsson@xxxxxxxxx>; Fijalkowski, Maciej
> <maciej.fijalkowski@xxxxxxxxx>; jonathan.lemon@xxxxxxxxx;
> davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; Sarkar, Tirthendu
> <tirthendu.sarkar@xxxxxxxxx>
> 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 <tushar.vyavahare@xxxxxxxxx>
> > ---
> > 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().
