* [PATCH bpf-next] selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata
@ 2023-01-25 22:32 Stanislav Fomichev
2023-01-26 1:12 ` Martin KaFai Lau
2023-01-27 9:34 ` Jesper Dangaard Brouer
0 siblings, 2 replies; 4+ messages in thread
From: Stanislav Fomichev @ 2023-01-25 22:32 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, Jesper Dangaard Brouer
The existing timestamping_enable() is a no-op because it applies
to the socket-related path that we are not verifying here
anymore. (but still leaving the code around hoping we can
have xdp->skb path verified here as well)
poll: 1 (0)
xsk_ring_cons__peek: 1
0xf64788: rx_desc[0]->addr=100000000008000 addr=8100 comp_addr=8000
rx_hash: 3697961069
rx_timestamp: 1674657672142214773 (sec:1674657672.1422)
XDP RX-time: 1674657709561774876 (sec:1674657709.5618) delta sec:37.4196
AF_XDP time: 1674657709561871034 (sec:1674657709.5619) delta
sec:0.0001 (96.158 usec)
0xf64788: complete idx=8 addr=8000
Also, maybe something to archive here, see [0] for Jesper's note
about NIC vs host clock delta.
0: https://lore.kernel.org/bpf/f3a116dc-1b14-3432-ad20-a36179ef0608@redhat.com/
Fixes: 297a3f124155 ("selftests/bpf: Simple program to dump XDP RX metadata")
Reported-by: Jesper Dangaard Brouer <jbrouer@redhat.com>
Tested-by: Jesper Dangaard Brouer <jbrouer@redhat.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/xdp_hw_metadata.c | 28 ++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
index 0008f0f239e8..dc899c53db5e 100644
--- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
+++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
@@ -24,6 +24,7 @@
#include <linux/net_tstamp.h>
#include <linux/udp.h>
#include <linux/sockios.h>
+#include <linux/net_tstamp.h>
#include <sys/mman.h>
#include <net/if.h>
#include <poll.h>
@@ -278,13 +279,36 @@ static int rxq_num(const char *ifname)
ret = ioctl(fd, SIOCETHTOOL, &ifr);
if (ret < 0)
- error(-1, errno, "socket");
+ error(-1, errno, "ioctl(SIOCETHTOOL)");
close(fd);
return ch.rx_count + ch.combined_count;
}
+static void hwtstamp_enable(const char *ifname)
+{
+ struct hwtstamp_config cfg = {
+ .rx_filter = HWTSTAMP_FILTER_ALL,
+ };
+
+ struct ifreq ifr = {
+ .ifr_data = (void *)&cfg,
+ };
+ strcpy(ifr.ifr_name, ifname);
+ int fd, ret;
+
+ fd = socket(AF_UNIX, SOCK_DGRAM, 0);
+ if (fd < 0)
+ error(-1, errno, "socket");
+
+ ret = ioctl(fd, SIOCSHWTSTAMP, &ifr);
+ if (ret < 0)
+ error(-1, errno, "ioctl(SIOCSHWTSTAMP)");
+
+ close(fd);
+}
+
static void cleanup(void)
{
LIBBPF_OPTS(bpf_xdp_attach_opts, opts);
@@ -341,6 +365,8 @@ int main(int argc, char *argv[])
printf("rxq: %d\n", rxq);
+ hwtstamp_enable(ifname);
+
rx_xsk = malloc(sizeof(struct xsk) * rxq);
if (!rx_xsk)
error(-1, ENOMEM, "malloc");
--
2.39.1.456.gfc5497dd1b-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata
2023-01-25 22:32 [PATCH bpf-next] selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata Stanislav Fomichev
@ 2023-01-26 1:12 ` Martin KaFai Lau
2023-01-26 17:31 ` Stanislav Fomichev
2023-01-27 9:34 ` Jesper Dangaard Brouer
1 sibling, 1 reply; 4+ messages in thread
From: Martin KaFai Lau @ 2023-01-26 1:12 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: ast, daniel, andrii, song, yhs, john.fastabend, kpsingh, haoluo,
jolsa, Jesper Dangaard Brouer, bpf
On 1/25/23 2:32 PM, Stanislav Fomichev wrote:
> The existing timestamping_enable() is a no-op because it applies
> to the socket-related path that we are not verifying here
> anymore. (but still leaving the code around hoping we can
> have xdp->skb path verified here as well)
>
> poll: 1 (0)
> xsk_ring_cons__peek: 1
> 0xf64788: rx_desc[0]->addr=100000000008000 addr=8100 comp_addr=8000
> rx_hash: 3697961069
> rx_timestamp: 1674657672142214773 (sec:1674657672.1422)
> XDP RX-time: 1674657709561774876 (sec:1674657709.5618) delta sec:37.4196
> AF_XDP time: 1674657709561871034 (sec:1674657709.5619) delta
> sec:0.0001 (96.158 usec)
> 0xf64788: complete idx=8 addr=8000
>
> Also, maybe something to archive here, see [0] for Jesper's note
> about NIC vs host clock delta.
>
> 0: https://lore.kernel.org/bpf/f3a116dc-1b14-3432-ad20-a36179ef0608@redhat.com/
>
> Fixes: 297a3f124155 ("selftests/bpf: Simple program to dump XDP RX metadata")
> Reported-by: Jesper Dangaard Brouer <jbrouer@redhat.com>
> Tested-by: Jesper Dangaard Brouer <jbrouer@redhat.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> tools/testing/selftests/bpf/xdp_hw_metadata.c | 28 ++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> index 0008f0f239e8..dc899c53db5e 100644
> --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
> +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> @@ -24,6 +24,7 @@
> #include <linux/net_tstamp.h>
> #include <linux/udp.h>
> #include <linux/sockios.h>
> +#include <linux/net_tstamp.h>
> #include <sys/mman.h>
> #include <net/if.h>
> #include <poll.h>
> @@ -278,13 +279,36 @@ static int rxq_num(const char *ifname)
>
> ret = ioctl(fd, SIOCETHTOOL, &ifr);
> if (ret < 0)
> - error(-1, errno, "socket");
> + error(-1, errno, "ioctl(SIOCETHTOOL)");
>
> close(fd);
>
> return ch.rx_count + ch.combined_count;
> }
>
> +static void hwtstamp_enable(const char *ifname)
> +{
> + struct hwtstamp_config cfg = {
> + .rx_filter = HWTSTAMP_FILTER_ALL,
> + };
> +
> + struct ifreq ifr = {
> + .ifr_data = (void *)&cfg,
> + };
> + strcpy(ifr.ifr_name, ifname);
> + int fd, ret;
> +
> + fd = socket(AF_UNIX, SOCK_DGRAM, 0);
> + if (fd < 0)
> + error(-1, errno, "socket");
> +
> + ret = ioctl(fd, SIOCSHWTSTAMP, &ifr);
> + if (ret < 0)
> + error(-1, errno, "ioctl(SIOCSHWTSTAMP)");
> +
> + close(fd);
> +}
> +
> static void cleanup(void)
> {
> LIBBPF_OPTS(bpf_xdp_attach_opts, opts);
> @@ -341,6 +365,8 @@ int main(int argc, char *argv[])
>
> printf("rxq: %d\n", rxq);
>
> + hwtstamp_enable(ifname);
Will it to restore to the earlier setting when the process exits?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata
2023-01-26 1:12 ` Martin KaFai Lau
@ 2023-01-26 17:31 ` Stanislav Fomichev
0 siblings, 0 replies; 4+ messages in thread
From: Stanislav Fomichev @ 2023-01-26 17:31 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: ast, daniel, andrii, song, yhs, john.fastabend, kpsingh, haoluo,
jolsa, Jesper Dangaard Brouer, bpf
On Wed, Jan 25, 2023 at 5:12 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 1/25/23 2:32 PM, Stanislav Fomichev wrote:
> > The existing timestamping_enable() is a no-op because it applies
> > to the socket-related path that we are not verifying here
> > anymore. (but still leaving the code around hoping we can
> > have xdp->skb path verified here as well)
> >
> > poll: 1 (0)
> > xsk_ring_cons__peek: 1
> > 0xf64788: rx_desc[0]->addr=100000000008000 addr=8100 comp_addr=8000
> > rx_hash: 3697961069
> > rx_timestamp: 1674657672142214773 (sec:1674657672.1422)
> > XDP RX-time: 1674657709561774876 (sec:1674657709.5618) delta sec:37.4196
> > AF_XDP time: 1674657709561871034 (sec:1674657709.5619) delta
> > sec:0.0001 (96.158 usec)
> > 0xf64788: complete idx=8 addr=8000
> >
> > Also, maybe something to archive here, see [0] for Jesper's note
> > about NIC vs host clock delta.
> >
> > 0: https://lore.kernel.org/bpf/f3a116dc-1b14-3432-ad20-a36179ef0608@redhat.com/
> >
> > Fixes: 297a3f124155 ("selftests/bpf: Simple program to dump XDP RX metadata")
> > Reported-by: Jesper Dangaard Brouer <jbrouer@redhat.com>
> > Tested-by: Jesper Dangaard Brouer <jbrouer@redhat.com>
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > tools/testing/selftests/bpf/xdp_hw_metadata.c | 28 ++++++++++++++++++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> > index 0008f0f239e8..dc899c53db5e 100644
> > --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
> > +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> > @@ -24,6 +24,7 @@
> > #include <linux/net_tstamp.h>
> > #include <linux/udp.h>
> > #include <linux/sockios.h>
> > +#include <linux/net_tstamp.h>
> > #include <sys/mman.h>
> > #include <net/if.h>
> > #include <poll.h>
> > @@ -278,13 +279,36 @@ static int rxq_num(const char *ifname)
> >
> > ret = ioctl(fd, SIOCETHTOOL, &ifr);
> > if (ret < 0)
> > - error(-1, errno, "socket");
> > + error(-1, errno, "ioctl(SIOCETHTOOL)");
> >
> > close(fd);
> >
> > return ch.rx_count + ch.combined_count;
> > }
> >
> > +static void hwtstamp_enable(const char *ifname)
> > +{
> > + struct hwtstamp_config cfg = {
> > + .rx_filter = HWTSTAMP_FILTER_ALL,
> > + };
> > +
> > + struct ifreq ifr = {
> > + .ifr_data = (void *)&cfg,
> > + };
> > + strcpy(ifr.ifr_name, ifname);
> > + int fd, ret;
> > +
> > + fd = socket(AF_UNIX, SOCK_DGRAM, 0);
> > + if (fd < 0)
> > + error(-1, errno, "socket");
> > +
> > + ret = ioctl(fd, SIOCSHWTSTAMP, &ifr);
> > + if (ret < 0)
> > + error(-1, errno, "ioctl(SIOCSHWTSTAMP)");
> > +
> > + close(fd);
> > +}
> > +
> > static void cleanup(void)
> > {
> > LIBBPF_OPTS(bpf_xdp_attach_opts, opts);
> > @@ -341,6 +365,8 @@ int main(int argc, char *argv[])
> >
> > printf("rxq: %d\n", rxq);
> >
> > + hwtstamp_enable(ifname);
>
> Will it to restore to the earlier setting when the process exits?
Nope, do you think we should? Probably a good idea, let me add that...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata
2023-01-25 22:32 [PATCH bpf-next] selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata Stanislav Fomichev
2023-01-26 1:12 ` Martin KaFai Lau
@ 2023-01-27 9:34 ` Jesper Dangaard Brouer
1 sibling, 0 replies; 4+ messages in thread
From: Jesper Dangaard Brouer @ 2023-01-27 9:34 UTC (permalink / raw)
To: Stanislav Fomichev, bpf
Cc: brouer, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, haoluo, jolsa, Jesper Dangaard Brouer
On 25/01/2023 23.32, Stanislav Fomichev wrote:
> The existing timestamping_enable() is a no-op because it applies
> to the socket-related path that we are not verifying here
> anymore. (but still leaving the code around hoping we can
> have xdp->skb path verified here as well)
>
> poll: 1 (0)
> xsk_ring_cons__peek: 1
> 0xf64788: rx_desc[0]->addr=100000000008000 addr=8100 comp_addr=8000
> rx_hash: 3697961069
> rx_timestamp: 1674657672142214773 (sec:1674657672.1422)
> XDP RX-time: 1674657709561774876 (sec:1674657709.5618) delta sec:37.4196
> AF_XDP time: 1674657709561871034 (sec:1674657709.5619) delta sec:0.0001 (96.158 usec)
This output contains some extra output data, which is not part of
current upstream.
I will soon submit an RFC-patch with this extra output to discuss and
figure out what timestamp type we want/expect HW to provide.
> 0xf64788: complete idx=8 addr=8000
>
> Also, maybe something to archive here, see [0] for Jesper's note
> about NIC vs host clock delta.
>
> 0: https://lore.kernel.org/bpf/f3a116dc-1b14-3432-ad20-a36179ef0608@redhat.com/
>
--Jesper
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-27 9:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 22:32 [PATCH bpf-next] selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata Stanislav Fomichev
2023-01-26 1:12 ` Martin KaFai Lau
2023-01-26 17:31 ` Stanislav Fomichev
2023-01-27 9:34 ` Jesper Dangaard Brouer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.