From: Paul Chavent <hidden> Date: 2012-12-12 15:33:31
This patch allow to generate tx timestamps of packets sent by the packet mmap interface.
Actually, you can't get tx timestamps with the sample code below.
I wonder if my current implementation is good. And if not, how should i get the timestamps ?
Wouldn't be a good idea to put timestamps in the ring buffer frame before give it back to the user ?
Thanks for your comments.
/* BEGIN OF SAMPLE CODE */
struct timespec ts = {0,0};
struct sockaddr from_addr;
static uint8_t tmp_data[256];
struct iovec msg_iov = {tmp_data, sizeof(tmp_data)};
static uint8_t cmsg_buff[256];
struct msghdr msghdr = {&from_addr, sizeof(from_addr),
&msg_iov, 1,
cmsg_buff, sizeof(cmsg_buff),
0};
ssize_t err = recvmsg(itf->sock_fd, &msghdr, MSG_ERRQUEUE);
if(err < 0)
{
perror("recvmsg failed");
return -1;
}
struct cmsghdr *cmsg;
for(cmsg = CMSG_FIRSTHDR(&msghdr); cmsg != NULL; cmsg = CMSG_NXTHDR(&msghdr, cmsg))
{
if(cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_TIMESTAMPING)
{
ts = *(struct timespec *)CMSG_DATA(cmsg);
fprintf(stderr, "SCM_TIMESTAMPING available\n");
}
else if (cmsg->cmsg_level == SOL_PACKET && cmsg->cmsg_type == PACKET_TX_TIMESTAMP)
{
ts = *(struct timespec *)CMSG_DATA(cmsg);
fprintf(stderr, "PACKET_TX_TIMESTAMP available\n");
}
}
/* END OF SAMPLE CODE */
Signed-off-by: Paul Chavent <redacted>
---
net/packet/af_packet.c | 4 ++++
1 file changed, 4 insertions(+)
From: Paul Chavent <hidden> Date: 2012-12-13 07:15:38
After a sendmsg, we have to call recvmsg on the ERRQUEUE to get
timestamp. I find that unfortunate indeed...
So this patch fix the tx timestamping (that take place in sendmsg), in
order to be able to get timestamp (via recvmsg).
This seems suboptimal to me, that why i also ask if it wouldn't be
possible to put the timestamp in the ring buffer frame before give it
back to user.
Thanks for your reading.
On 12/12/2012 08:23 PM, David Miller wrote:
You're changing the code that handles sendmsg() and then wondering why
a recvmsg() call doesn't provide a timestamp.
From: Richard Cochran <richardcochran@gmail.com> Date: 2012-12-13 13:29:29
On Wed, Dec 12, 2012 at 04:29:25PM +0100, Paul Chavent wrote:
This patch allow to generate tx timestamps of packets sent by the packet mmap interface.
Actually, you can't get tx timestamps with the sample code below.
I wonder if my current implementation is good. And if not, how should i get the timestamps ?
In order for time stamps to appear, somebody has to call
skb_tx_timestamp() ...
From: Paul Chavent <hidden> Date: 2012-12-13 16:16:35
Hello.
On 12/13/2012 02:29 PM, Richard Cochran wrote:
On Wed, Dec 12, 2012 at 04:29:25PM +0100, Paul Chavent wrote:
quoted
This patch allow to generate tx timestamps of packets sent by the packet mmap interface.
Actually, you can't get tx timestamps with the sample code below.
I wonder if my current implementation is good. And if not, how should i get the timestamps ?
In order for time stamps to appear, somebody has to call
skb_tx_timestamp() ...
Yes. "Somebody" means "the hardware driver" after completing xmit.
That's true ?
>
> and this call is only setting some flags.
Yes, it only sets some flags. I thought that those flags was required by
the skb_tx_timestamp() in order to make the appropriate timestamping
(hardware, software, etc).
So in order to have tx timestamp that work, both calls are needed ?
Why sock_tx_timestamp is called in packet_fill_skb and
packet_sendmsg_spkt and not in tpacket_fill_skb ?
Why i can retrieve timestamps when i add this call ?
From: Richard Cochran <richardcochran@gmail.com> Date: 2012-12-13 18:17:44
On Thu, Dec 13, 2012 at 05:13:56PM +0100, Paul Chavent wrote:
quoted
In order for time stamps to appear, somebody has to call
skb_tx_timestamp() ...
Yes. "Somebody" means "the hardware driver" after completing xmit.
That's true ?
Yes, the MAC driver must call this helper function, but not many
drivers do this yet. You didn't say which MAC driver you are using and
whether it supports Tx SO_TIMESTAMPING or not.
Yes, it only sets some flags. I thought that those flags was
required by the skb_tx_timestamp() in order to make the appropriate
timestamping (hardware, software, etc).
So in order to have tx timestamp that work, both calls are needed ?
Yes.
Why sock_tx_timestamp is called in packet_fill_skb and
packet_sendmsg_spkt and not in tpacket_fill_skb ?
Why i can retrieve timestamps when i add this call ?
Sorry, I don't know much about packet mmap. Last time I tried it, some
years ago, it wasn't really working.
Richard
From: Paul Chavent <hidden> Date: 2012-12-14 07:59:59
On 12/13/2012 07:17 PM, Richard Cochran wrote:
On Thu, Dec 13, 2012 at 05:13:56PM +0100, Paul Chavent wrote:
quoted
quoted
In order for time stamps to appear, somebody has to call
skb_tx_timestamp() ...
Yes. "Somebody" means "the hardware driver" after completing xmit.
That's true ?
Yes, the MAC driver must call this helper function, but not many
drivers do this yet. You didn't say which MAC driver you are using and
whether it supports Tx SO_TIMESTAMPING or not.
I'm using the uml net device (which recently gains tx timestamping),
e1000e (wich seems to support it according to my tests), and arm macb
(wich seems to support it too).
quoted
Yes, it only sets some flags. I thought that those flags was
required by the skb_tx_timestamp() in order to make the appropriate
timestamping (hardware, software, etc).
So in order to have tx timestamp that work, both calls are needed ?
Yes.
quoted
Why sock_tx_timestamp is called in packet_fill_skb and
packet_sendmsg_spkt and not in tpacket_fill_skb ?
Why i can retrieve timestamps when i add this call ?
Sorry, I don't know much about packet mmap. Last time I tried it, some
years ago, it wasn't really working.
I haven't measured the performance, but it works for me (however, not on
my arm platfrom yet).
Richard
The af_packet implementation contains 3 "paths" for packets. Perhaps i'm
a bit confused by its complexity.
Paul.
From: Paul Chavent <hidden> Date: 2013-04-09 10:54:29
On 12/13/2012 07:17 PM, Richard Cochran wrote:
On Thu, Dec 13, 2012 at 05:13:56PM +0100, Paul Chavent wrote:
quoted
quoted
In order for time stamps to appear, somebody has to call
skb_tx_timestamp() ...
Yes. "Somebody" means "the hardware driver" after completing xmit.
That's true ?
Yes, the MAC driver must call this helper function, but not many
drivers do this yet. You didn't say which MAC driver you are using and
whether it supports Tx SO_TIMESTAMPING or not.
quoted
Yes, it only sets some flags. I thought that those flags was
required by the skb_tx_timestamp() in order to make the appropriate
timestamping (hardware, software, etc).
So in order to have tx timestamp that work, both calls are needed ?
Yes.
quoted
Why sock_tx_timestamp is called in packet_fill_skb and
packet_sendmsg_spkt and not in tpacket_fill_skb ?
Why i can retrieve timestamps when i add this call ?
Sorry, I don't know much about packet mmap. Last time I tried it, some
years ago, it wasn't really working.
Richard
Hi.
Would it be possible that the packet mmap maintainers give their opinion
on this thread please ?
Regards.
Paul.
From: Richard Cochran <richardcochran@gmail.com> Date: 2013-04-09 13:15:43
On Tue, Apr 09, 2013 at 12:42:57PM +0200, Paul Chavent wrote:
Would it be possible that the packet mmap maintainers give their
opinion on this thread please ?
I was digging around, trying to understand whether libpcap can get HW
time stamps via the packet_mmap interface, and I found this.
commit 614f60fa9d73a9e8fdff3df83381907fea7c5649
Author: Scott McMillan [off-list ref]
Date: Wed Jun 2 05:53:56 2010 -0700
packet_mmap: expose hw packet timestamps to network packet capture utilities
Maybe you could ask Scott for help?
[ It looks to me like that patch is kinda useless, since the user has
no way to tell whether the time stamps are from HW or SW. ]
HTH,
Richard
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-13 18:34:21
On Thu, Dec 13, 2012 at 11:13 AM, Paul Chavent [off-list ref] wrote:
Hello.
On 12/13/2012 02:29 PM, Richard Cochran wrote:
quoted
On Wed, Dec 12, 2012 at 04:29:25PM +0100, Paul Chavent wrote:
quoted
This patch allow to generate tx timestamps of packets sent by the packet
mmap interface.
Actually, you can't get tx timestamps with the sample code below.
I wonder if my current implementation is good. And if not, how should i
get the timestamps ?
In order for time stamps to appear, somebody has to call
skb_tx_timestamp() ...
Yes. "Somebody" means "the hardware driver" after completing xmit. That's
true ?
Yes, it only sets some flags. I thought that those flags was required by the
skb_tx_timestamp() in order to make the appropriate timestamping (hardware,
software, etc).
So in order to have tx timestamp that work, both calls are needed ?
Yes.
Why sock_tx_timestamp is called in packet_fill_skb and packet_sendmsg_spkt
and not in tpacket_fill_skb ?
Good point. From what I can tell, if you add it, the existing error
queue mechanism should work fine. At a glance, your code looks fine,
too, although I haven't tested it.
Why i can retrieve timestamps when i add this call ?
I actually wondered it if would be possible to return the timestamps
in the ring itself. It is, but requires two additional changes:
writing the timestamp to orig_skb->tstamp in skb_tstamp_tx and then
writing it into the ring in tpacket_destruct_skb. The first change is
a bit odd, as the orig_skb is usually freed shortly after
skb_tstamp_tx is called. Still, I verified that it works and the patch
should also make your errorqueue-based code work, as it inserts the
sock_tx_timestamp. Will send it for review following this.
quoted
HTH,
Richard
Thank for your help.
Paul.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-13 18:56:55
When transmit timestamping is enabled at the socket level, have
writes to a PACKET_TX_RING record a timestamp for the generated
skbuffs. Tx timestamps are always looped to the application over
the socket error queue.
The patch also loops software timestamps back into the ring.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 2 +-
net/packet/af_packet.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 1 deletion(-)
@@ -341,6 +341,45 @@ static int __packet_get_status(struct packet_sock *po, void *frame)}}+staticvoid__packet_set_timestamp(structpacket_sock*po,void*frame,+ktime_ttstamp)+{+structtpacket_hdr*h1;+structtpacket2_hdr*h2;+structtimespects;++if(!tstamp.tv64||!sock_flag(&po->sk,SOCK_TIMESTAMPING_SOFTWARE))+return;++ts=ktime_to_timespec(tstamp);++switch(po->tp_version){+caseTPACKET_V1:+h1=frame;+h1->tp_sec=ts.tv_sec;+h1->tp_usec=ts.tv_nsec/NSEC_PER_USEC;++flush_dcache_page(pgv_to_page(&h1->tp_sec));+flush_dcache_page(pgv_to_page(&h1->tp_usec));+break;+caseTPACKET_V2:+h2=frame;+h2->tp_sec=ts.tv_sec;+h2->tp_nsec=ts.tv_nsec;++flush_dcache_page(pgv_to_page(&h2->tp_sec));+flush_dcache_page(pgv_to_page(&h2->tp_nsec));+break;+caseTPACKET_V3:+default:+WARN(1,"TPACKET version not supported.\n");+BUG();+}+++smp_wmb();+}+staticvoid*packet_lookup_frame(structpacket_sock*po,structpacket_ring_buffer*rb,unsignedintposition,
From: Daniel Borkmann <hidden> Date: 2013-04-13 22:19:39
On 04/13/2013 08:56 PM, Willem de Bruijn wrote:
When transmit timestamping is enabled at the socket level, have
writes to a PACKET_TX_RING record a timestamp for the generated
skbuffs. Tx timestamps are always looped to the application over
the socket error queue.
Nitpick: if so, then this should go to net-next (subject line).
The patch also loops software timestamps back into the ring.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 2 +-
net/packet/af_packet.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 1 deletion(-)
Hmm, so in case nobody wants to use timestamping on TX (which might be the majority
of people), we have to go through those 3 additional if statements in sock_tx_timestamp()
each time? Shouldn't we rather make the TX_RING faster? ;-)
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-14 00:00:56
On Sat, Apr 13, 2013 at 6:18 PM, Daniel Borkmann [off-list ref] wrote:
On 04/13/2013 08:56 PM, Willem de Bruijn wrote:
quoted
When transmit timestamping is enabled at the socket level, have
writes to a PACKET_TX_RING record a timestamp for the generated
skbuffs. Tx timestamps are always looped to the application over
the socket error queue.
Nitpick: if so, then this should go to net-next (subject line).
Thanks.
quoted
The patch also loops software timestamps back into the ring.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 2 +-
net/packet/af_packet.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 1 deletion(-)
Hmm, so in case nobody wants to use timestamping on TX (which might be the
majority
of people), we have to go through those 3 additional if statements in
sock_tx_timestamp()
each time? Shouldn't we rather make the TX_RING faster? ;-)
A static key, similar to netstamp_needed, might make it cheaper, but
may be more complexity than warranted for a corner case. Simpler is
testing only the software timestamp, and in tpacket_fill_skb, where
the sk struct is already accessed. I don't feel strongly about merging
given the performance trade-off: just wanted to see if it worked.
Happy to resubmit either revision (or a better idea).
Hmm, not sure, but could we also flush the dcache only once?
Indeed, I truly hope that headers never straddle pages.
I should have checked the alignment restrictions on frames. Frames
must be a multiple of 16 B as well as larger than the header (obviously),
so this can indeed never happen.
Hmm, not sure, but could we also flush the dcache only once?
Indeed, I truly hope that headers never straddle pages.
I should have checked the alignment restrictions on frames. Frames
must be a multiple of 16 B as well as larger than the header (obviously),
so this can indeed never happen.
Actually, 48 B is a multiple of 16, so should be accepted, and 85
frames on a page leaves half a frame for the next. I'll check whether
this is right. Even if so, it would still not matter for these time
offsets, as they start at 16 or 20 B offset from the start of the
frame.
Hmm, not sure, but could we also flush the dcache only once?
Indeed, I truly hope that headers never straddle pages.
I should have checked the alignment restrictions on frames. Frames
must be a multiple of 16 B as well as larger than the header (obviously),
so this can indeed never happen.
Actually, 48 B is a multiple of 16, so should be accepted, and 85
frames on a page leaves half a frame for the next. I'll check whether
this is right. Even if so, it would still not matter for these time
offsets, as they start at 16 or 20 B offset from the start of the
frame.
48 B is too small (Because less than TPACKET_HDRLEN), but it can
be triggered with 80 B frames. Daniel, thanks for submitting the
selftest: both the timestamp and this alignment question were now very
easy to test by just changing a few lines in your code.
Hmm, not sure, but could we also flush the dcache only once?
Indeed, I truly hope that headers never straddle pages.
I should have checked the alignment restrictions on frames. Frames
must be a multiple of 16 B as well as larger than the header (obviously),
so this can indeed never happen.
Actually, 48 B is a multiple of 16, so should be accepted, and 85
frames on a page leaves half a frame for the next. I'll check whether
this is right. Even if so, it would still not matter for these time
offsets, as they start at 16 or 20 B offset from the start of the
frame.
48 B is too small (Because less than TPACKET_HDRLEN), but it can
be triggered with 80 B frames. Daniel, thanks for submitting the
selftest: both the timestamp and this alignment question were now very
easy to test by just changing a few lines in your code.
From: Daniel Borkmann <hidden> Date: 2013-04-14 10:53:22
On 04/14/2013 02:00 AM, Willem de Bruijn wrote:
On Sat, Apr 13, 2013 at 6:18 PM, Daniel Borkmann [off-list ref] wrote:
quoted
On 04/13/2013 08:56 PM, Willem de Bruijn wrote:
quoted
When transmit timestamping is enabled at the socket level, have
writes to a PACKET_TX_RING record a timestamp for the generated
skbuffs. Tx timestamps are always looped to the application over
the socket error queue.
Nitpick: if so, then this should go to net-next (subject line).
Thanks.
quoted
quoted
The patch also loops software timestamps back into the ring.
Also v2 should probably contain a ``Reported-by''.
While going a bit more through the code, I'm wondering .. if we want to support
TX timestamps, could we also support SW _and_ HW timestamps e.g. similar as in
sock_recv_timestamp()? I'm asking, because we already allow setting the flags
for it via sock_tx_timestamp(). This might be good, if possible.
Paul, since you've reported / requested this, your use case would be to fill
the ring, trigger a sendto() and then loop through all the frames to check the
tx timestamps for your custom protocol mockup, then fill the returned frames
again, etc.?
From: Richard Cochran <richardcochran@gmail.com> Date: 2013-04-14 13:07:40
On Sun, Apr 14, 2013 at 12:52:16PM +0200, Daniel Borkmann wrote:
While going a bit more through the code, I'm wondering .. if we want to support
TX timestamps, could we also support SW _and_ HW timestamps e.g. similar as in
sock_recv_timestamp()? I'm asking, because we already allow setting the flags
for it via sock_tx_timestamp(). This might be good, if possible.
And while you are at it, you could also fix the receive code.
As it stand now, it is fairly useless, since there is no way for user
space to tell which kind of time stamp has been reported. In fact, the
code will silently intermingle hardware and software time stamps. That
is surely a mean trick to play on the users.
Thanks,
Richard
You said that "the orig_skb is usually freed shortly after
skb_tstamp_tx is called".
So i suppose that if you had coded it, that's because this orig_skb is
the one that is given to the skb destructor. So when you call
__packet_set_timestamp, in tpacket_destruct_skb, you get this timestamp
? Am i right ?
Why we couldn't call
*skb_hwtstamps(orig_skb) = *skb_hwtstamps(skb) = *hwtstamps;
in order to get the hardware timestamping too ?
Thank for your help.
Paul.
From: Paul Chavent <hidden> Date: 2013-04-15 07:41:54
On 04/14/2013 03:07 PM, Richard Cochran wrote:
On Sun, Apr 14, 2013 at 12:52:16PM +0200, Daniel Borkmann wrote:
quoted
While going a bit more through the code, I'm wondering .. if we want to support
TX timestamps, could we also support SW _and_ HW timestamps e.g. similar as in
sock_recv_timestamp()? I'm asking, because we already allow setting the flags
for it via sock_tx_timestamp(). This might be good, if possible.
And while you are at it, you could also fix the receive code.
As it stand now, it is fairly useless, since there is no way for user
space to tell which kind of time stamp has been reported. In fact, the
code will silently intermingle hardware and software time stamps. That
is surely a mean trick to play on the users.
Isn't it the one that the user ask with setsockopt(fd, SOL_PACKET,
PACKET_TIMESTAMP, ×tamping, sizeof(timestamping)) ?
However, i wonder why you added an other sockopt that do the same thing
as SOL_SOCKET/SO_TIMESTAMPING sockopt ?
Paul.
From: Paul Chavent <hidden> Date: 2013-04-15 15:46:08
On 04/14/2013 12:52 PM, Daniel Borkmann wrote:
Paul, since you've reported / requested this, your use case would be to
fill
the ring, trigger a sendto() and then loop through all the frames to
check the
tx timestamps for your custom protocol mockup, then fill the returned
frames
again, etc.?
I've run a test that create (and setup) a tx ring buffer with 8 frames,
then fill the frames payload, then call sendto.
I've checked the timestamp by two means :
- check the timestamp in the tx ring after the status became
TP_STATUS_AVAILABLE again,
- by issuing 8 recvmsg on the ERRQUEUE.
Both timestamp are coherents and increment themselves.
The tests has been done with a UML kernel, and with software timestamps.
From my point of view, it seems usable.
However, I've found one strange behavior. I must call recvmsg as many
times as i have submitted a frame, or not at all. If i only pop the
ERRQUEUE for one or two message for instance, the next call to sentdo
fails with "No message of desired type" (errno 42).
Paul.
You said that "the orig_skb is usually freed shortly after
skb_tstamp_tx is called".
So i suppose that if you had coded it, that's because this orig_skb is the
one that is given to the skb destructor. So when you call
__packet_set_timestamp, in tpacket_destruct_skb, you get this timestamp ? Am
i right ?
Why we couldn't call
*skb_hwtstamps(orig_skb) = *skb_hwtstamps(skb) = *hwtstamps;
in order to get the hardware timestamping too ?
Probably. That would also answer Daniel's question to the same
functionality. I'm a bit less familiar with the hardware path, so will
read up before just submitting it.
From: Richard Cochran <richardcochran@gmail.com> Date: 2013-04-15 16:56:16
On Mon, Apr 15, 2013 at 09:37:30AM +0200, Paul Chavent wrote:
On 04/14/2013 03:07 PM, Richard Cochran wrote:
quoted
As it stand now, it is fairly useless, since there is no way for user
space to tell which kind of time stamp has been reported. In fact, the
code will silently intermingle hardware and software time stamps. That
is surely a mean trick to play on the users.
Isn't it the one that the user ask with setsockopt(fd, SOL_PACKET,
PACKET_TIMESTAMP, ×tamping, sizeof(timestamping)) ?
No, not necessarily. Look at the code in net/packet/af_packet.c.
if ((po->tp_tstamp & SOF_TIMESTAMPING_SYS_HARDWARE)
&& shhwtstamps->syststamp.tv64)
ts = ktime_to_timespec(shhwtstamps->syststamp);
else if ((po->tp_tstamp & SOF_TIMESTAMPING_RAW_HARDWARE)
&& shhwtstamps->hwtstamp.tv64)
ts = ktime_to_timespec(shhwtstamps->hwtstamp);
else if (skb->tstamp.tv64)
ts = ktime_to_timespec(skb->tstamp);
else
getnstimeofday(&ts);
What happens if RAW is requested, but no HW time stamp is available?
However, i wonder why you added an other sockopt that do the same
thing as SOL_SOCKET/SO_TIMESTAMPING sockopt ?
Not sure what you mean. I did not add the SOL_PACKET socket option.
Thanks,
Richard
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-15 16:59:56
On Mon, Apr 15, 2013 at 3:37 AM, Paul Chavent [off-list ref] wrote:
On 04/14/2013 03:07 PM, Richard Cochran wrote:
quoted
On Sun, Apr 14, 2013 at 12:52:16PM +0200, Daniel Borkmann wrote:
quoted
While going a bit more through the code, I'm wondering .. if we want to
support
TX timestamps, could we also support SW _and_ HW timestamps e.g. similar
as in
sock_recv_timestamp()? I'm asking, because we already allow setting the
flags
for it via sock_tx_timestamp(). This might be good, if possible.
And while you are at it, you could also fix the receive code.
As it stand now, it is fairly useless, since there is no way for user
space to tell which kind of time stamp has been reported. In fact, the
code will silently intermingle hardware and software time stamps. That
is surely a mean trick to play on the users.
Interesting issue, but I'll leave that for a separate fix.
Isn't it the one that the user ask with setsockopt(fd, SOL_PACKET,
PACKET_TIMESTAMP, ×tamping, sizeof(timestamping)) ?
This option toggles whether, if an skbuff has a timestamp, it should
be written to the rx ring metadata. skbuffs can have multiple
timestamps, so it also determines which one is selected.
If I understand correctly, the problem that Richard refers to is that
when one socket requests rx timestamping, all rx skbuffs have to be
timestamped as it is not known early in the datapath to which socket
they will be enqueued. As a result, the timestamps in skbuffs depend
on the preferences of other active sockets. Correct me if I'm wrong.
If so, it may even vary during the lifetime of a packet socket, so a
getsockopt wouldn't help, either. Perhaps nothing short of a new field
in the frame header structure will. In which case it makes more sense
to support sending each type of timestamp independently, just like the
socker error queue mechanism. Adding yet another header format,
though?
However, i wonder why you added an other sockopt that do the same thing as
SOL_SOCKET/SO_TIMESTAMPING sockopt ?
Back to the patch. I'll revise with
- a small patch for the errqueue mechanism
- add hw timestamp pass-through in skb_tstamp_tx, if correct
- move sock_tx_timestamp where cache is warm
- update it to use only a single conditional in the common case: no
flags enabled
- a follow-up for the ring
- single flush_dcache_page, which is safe for current header layouts
on 32bit+ platforms
(it is also a noop on many of these)
Hmm, not sure, but could we also flush the dcache only once?
If it isn't a silly question, why is the dcache being flushed
here at all?
I'm not an expert on this, but have a look at
Documentation/cachetlb.txt, specifically the bits on this function and
the discussion of aliasing: on virtually indexed cache architectures,
the same physical address may be cached in multiple cachelines at the
same time. If I understand correctly, updating the kernel logical
address does not necessarily invalidate the user virtual cacheline for
the same physical memory.
Hmm, not sure, but could we also flush the dcache only once?
If it isn't a silly question, why is the dcache being flushed
here at all?
Anything that gets written into userspace mapped pages via the
kernel linear mapping of the pages must be D-cache flushed into
order to achieve coherency on cpus that have virtually indexed
caches.
From: Richard Cochran <richardcochran@gmail.com> Date: 2013-04-17 10:22:30
On Mon, Apr 15, 2013 at 12:59:26PM -0400, Willem de Bruijn wrote:
If I understand correctly, the problem that Richard refers to is that
when one socket requests rx timestamping, all rx skbuffs have to be
timestamped as it is not known early in the datapath to which socket
they will be enqueued. As a result, the timestamps in skbuffs depend
on the preferences of other active sockets. Correct me if I'm wrong.
No, that is not it.
Hardware time stamping needs to be globally enabled at the driver
level using the SIOCSHWTSTAMP [1] ioctl. The hardware has various
different abilities, usually one of:
- no time stamping
- some subset of PTP packets
- all packets
So depending on the mode, not all packets will receive a time
stamp. In addition, some of the hardware can only time stamp one
packet at a time. Furthermore, time stamps can get dropped in high
traffic situations.
The code in af_packet.c unconditionally delivers some sort of time
stamp, with an inexplicable fall back to gettimeofday. Thus, the user
space has no way of knowing what is being reported in the time stamp.
Thanks,
Richard
1. See Documentation/networking/timestamping.txt
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-19 21:58:14
v1->v2:
- move sock_tx_timestamp near other sk reads (warm cacheline)
- remove duplicate flush_dcache_page
- enable hardware timestamps reporting using the error queue (not ring)
- use new ktime_to_timespec_cond API
When transmit timestamping is enabled at the socket level, record a
timestamp on packets written to a PACKET_TX_RING. Tx timestamps are
always looped to the application over the socket error queue. Software
timestamps are also written back into the packet frame header in the
packet ring.
Reported-by: Paul Chavent <redacted>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 12 ++++++------
net/packet/af_packet.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 6 deletions(-)
@@ -339,6 +339,37 @@ static int __packet_get_status(struct packet_sock *po, void *frame)}}+staticvoid__packet_set_timestamp(structpacket_sock*po,void*frame,+ktime_ttstamp)+{+uniontpacket_uhdrh;+structtimespects;++if(!ktime_to_timespec_cond(tstamp,&ts)||+!sock_flag(&po->sk,SOCK_TIMESTAMPING_SOFTWARE))+return;++h.raw=frame;+switch(po->tp_version){+caseTPACKET_V1:+h.h1->tp_sec=ts.tv_sec;+h.h1->tp_usec=ts.tv_nsec/NSEC_PER_USEC;+break;+caseTPACKET_V2:+h.h2->tp_sec=ts.tv_sec;+h.h2->tp_nsec=ts.tv_nsec;+break;+caseTPACKET_V3:+default:+WARN(1,"TPACKET version not supported.\n");+BUG();+}++/* one flush is safe, as both fields always lie on the same cacheline */+flush_dcache_page(pgv_to_page(&h.h1->tp_sec));+smp_wmb();+}+staticvoid*packet_lookup_frame(structpacket_sock*po,structpacket_ring_buffer*rb,unsignedintposition,
From: Daniel Borkmann <hidden> Date: 2013-04-20 12:34:21
On 04/19/2013 11:51 PM, Willem de Bruijn wrote:
v1->v2:
- move sock_tx_timestamp near other sk reads (warm cacheline)
- remove duplicate flush_dcache_page
- enable hardware timestamps reporting using the error queue (not ring)
- use new ktime_to_timespec_cond API
Nitpick: probably this should rather go below the "---" line, so that this
will not show up in the commit message.
quoted hunk
When transmit timestamping is enabled at the socket level, record a
timestamp on packets written to a PACKET_TX_RING. Tx timestamps are
always looped to the application over the socket error queue. Software
timestamps are also written back into the packet frame header in the
packet ring.
Reported-by: Paul Chavent <redacted>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 12 ++++++------
net/packet/af_packet.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 6 deletions(-)
If we currently have the po->tp_tstamp member that was introduced for the RX_RING
part only, using it if possible for the TX_RING part as well would be good, at
least cleaner. Also the documentation [1] should receive a status update on this
feature, otherwise only Paul will use this feature and nobody else.
Not an expert in timestamping, but why can't we also allow hw timestamps but have
to use sw only? Would it be possible to reuse the tpacket_get_timestamp() function
that got recently introduced for this (while keeping sock_tx_timestamp() below on
TX path)?
Thanks.
[1] Documentation/networking/packet_mmap.txt
quoted hunk
+ h.raw = frame;
+ switch (po->tp_version) {
+ case TPACKET_V1:
+ h.h1->tp_sec = ts.tv_sec;
+ h.h1->tp_usec = ts.tv_nsec / NSEC_PER_USEC;
+ break;
+ case TPACKET_V2:
+ h.h2->tp_sec = ts.tv_sec;
+ h.h2->tp_nsec = ts.tv_nsec;
+ break;
+ case TPACKET_V3:
+ default:
+ WARN(1, "TPACKET version not supported.\n");
+ BUG();
+ }
+
+ /* one flush is safe, as both fields always lie on the same cacheline */
+ flush_dcache_page(pgv_to_page(&h.h1->tp_sec));
+ smp_wmb();
+}
+
static void *packet_lookup_frame(struct packet_sock *po,
struct packet_ring_buffer *rb,
unsigned int position,
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-21 02:30:44
On Sat, Apr 20, 2013 at 8:33 AM, Daniel Borkmann [off-list ref] wrote:
On 04/19/2013 11:51 PM, Willem de Bruijn wrote:
quoted
v1->v2:
- move sock_tx_timestamp near other sk reads (warm cacheline)
- remove duplicate flush_dcache_page
- enable hardware timestamps reporting using the error queue (not ring)
- use new ktime_to_timespec_cond API
Nitpick: probably this should rather go below the "---" line, so that this
will not show up in the commit message.
quoted
When transmit timestamping is enabled at the socket level, record a
timestamp on packets written to a PACKET_TX_RING. Tx timestamps are
always looped to the application over the socket error queue. Software
timestamps are also written back into the packet frame header in the
packet ring.
Reported-by: Paul Chavent <redacted>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 12 ++++++------
net/packet/af_packet.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 6 deletions(-)
If we currently have the po->tp_tstamp member that was introduced for the
RX_RING
part only, using it if possible for the TX_RING part as well would be good,
at
least cleaner.
Then the caller has to set both the SOL_SOCKET and SOL_PACKET
options. I'm not convinced that that is an improvement over using generic
socket tx timestamping option as is. How would you use it specifically?
To determine whether or not to write the values into the ring, or more
extensively?
Also the documentation [1] should receive a status update on
this
feature, otherwise only Paul will use this feature and nobody else.
Okay. I'll update the documentation. Speaking of which, it would be
great if someone could proofread the packet socket manpage update
patch (http://patchwork.ozlabs.org/patch/228709/)
Not an expert in timestamping, but why can't we also allow hw timestamps but
have
to use sw only?
To avoid getting into the same situation on tx that Richard pointed out
about rx: the ring can only communicate one timestamp and cannot
communicate which one it is. Better to be consistent in that case, I
think. I'm open to discussing this, though.
Would it be possible to reuse the tpacket_get_timestamp()
function
that got recently introduced for this (while keeping sock_tx_timestamp()
below on
TX path)?
Thanks.
[1] Documentation/networking/packet_mmap.txt
quoted
+ h.raw = frame;
+ switch (po->tp_version) {
+ case TPACKET_V1:
+ h.h1->tp_sec = ts.tv_sec;
+ h.h1->tp_usec = ts.tv_nsec / NSEC_PER_USEC;
+ break;
+ case TPACKET_V2:
+ h.h2->tp_sec = ts.tv_sec;
+ h.h2->tp_nsec = ts.tv_nsec;
+ break;
+ case TPACKET_V3:
+ default:
+ WARN(1, "TPACKET version not supported.\n");
+ BUG();
+ }
+
+ /* one flush is safe, as both fields always lie on the same
cacheline */
+ flush_dcache_page(pgv_to_page(&h.h1->tp_sec));
+ smp_wmb();
+}
+
static void *packet_lookup_frame(struct packet_sock *po,
struct packet_ring_buffer *rb,
unsigned int position,
From: Daniel Borkmann <hidden> Date: 2013-04-21 10:10:48
On 04/21/2013 04:30 AM, Willem de Bruijn wrote:
On Sat, Apr 20, 2013 at 8:33 AM, Daniel Borkmann [off-list ref] wrote:
[..]
quoted
On 04/19/2013 11:51 PM, Willem de Bruijn wrote:
If we currently have the po->tp_tstamp member that was introduced for the
RX_RING
part only, using it if possible for the TX_RING part as well would be good,
at
least cleaner.
Then the caller has to set both the SOL_SOCKET and SOL_PACKET
options. I'm not convinced that that is an improvement over using generic
socket tx timestamping option as is. How would you use it specifically?
To determine whether or not to write the values into the ring, or more
extensively?
I would use it in combination with tpacket_get_timestamp(). There, we do not
check for SOCK_TIMESTAMPING_SOFTWARE, but just fall back for skb->tstamp if
not null. The tpacket_get_timestamp() would need a small change though, in
that sense that i) getnstimeofday() is moved out of the function and ii) the
function will return a bool, if the ts variable was filled. Thus, in the RX
path if the function returns false, you do getnstimeofday() as fallback, and
in the TX path nothing needs to be done in __packet_set_timestamp() if the skb
was not timestamped.
That said, for the software timestamps, the caller would not need to do more
as with your current patch, for the hardware part, it's then the same afaik
as in the RX_RING.
Just to give some pseudo code (instead of the bool, we could also use an enum
that tells which timestamp was found, but more on that further below):
static bool tpacket_get_timestamp(struct sk_buff *skb, struct timespec *ts,
unsigned int flags)
{
struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
if (shhwtstamps) {
if ((flags & SOF_TIMESTAMPING_SYS_HARDWARE) &&
ktime_to_timespec_cond(shhwtstamps->syststamp, ts))
return true;
if ((flags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
ktime_to_timespec_cond(shhwtstamps->hwtstamp, ts))
return true;
}
if (ktime_to_timespec_cond(skb->tstamp, ts))
return true;
return false;
}
In tpacket_rcv():
...
if (!tpacket_get_timestamp(skb, &ts, po->tp_tstamp))
getnstimeofday(ts);
...
In __packet_set_timestamp():
...
if (!tpacket_get_timestamp(skb, &ts, po->tp_tstamp))
return;
...
quoted
Also the documentation [1] should receive a status update on
this
feature, otherwise only Paul will use this feature and nobody else.
Okay. I'll update the documentation. Speaking of which, it would be
great if someone could proofread the packet socket manpage update
patch (http://patchwork.ozlabs.org/patch/228709/)
Ok, I will have a look.
quoted
Not an expert in timestamping, but why can't we also allow hw timestamps but
have
to use sw only?
To avoid getting into the same situation on tx that Richard pointed out
about rx: the ring can only communicate one timestamp and cannot
communicate which one it is. Better to be consistent in that case, I
think. I'm open to discussing this, though.
Ok, I agree that it would be very useful to also state what timestamp is
being reported. Ideally, without introducing yet another tpacket version.
The only thing I can currently think of could be to use the tp_status bit
for that, then used by both RX_RING and TX_RING, e.g. ...
TP_STATUS_TS_SYS_HARDWARE (1 << 30)
TP_STATUS_TS_RAW_HARDWARE (1 << 31)
... although it would be a bit of waste to use 2 bits for that. If none of
them is set, it indicates that the timestamp is in software-only. This we
could do, if you don't have a better idea.
``Better to be consistent in that case'' would not be the current situation
with the patch, right? I mean, for RX_RING, we report one of those 3, for
TX_RING only the sw timestamp. So on one hand we might be consistent, but on
the other hand we're not. ;-) With the above proposal, we could tackle this
and stay consistent then. If you want, I could do that after your revised
patch gets accepted (with hw ts support). Let me know what you think of that.
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-21 16:43:17
On Sun, Apr 21, 2013 at 6:10 AM, Daniel Borkmann [off-list ref] wrote:
On 04/21/2013 04:30 AM, Willem de Bruijn wrote:
quoted
On Sat, Apr 20, 2013 at 8:33 AM, Daniel Borkmann [off-list ref]
wrote:
[..]
quoted
quoted
On 04/19/2013 11:51 PM, Willem de Bruijn wrote:
If we currently have the po->tp_tstamp member that was introduced for the
RX_RING
part only, using it if possible for the TX_RING part as well would be
good,
at
least cleaner.
Then the caller has to set both the SOL_SOCKET and SOL_PACKET
options. I'm not convinced that that is an improvement over using generic
socket tx timestamping option as is. How would you use it specifically?
To determine whether or not to write the values into the ring, or more
extensively?
I would use it in combination with tpacket_get_timestamp(). There, we do not
check for SOCK_TIMESTAMPING_SOFTWARE, but just fall back for skb->tstamp if
not null. The tpacket_get_timestamp() would need a small change though, in
that sense that i) getnstimeofday() is moved out of the function and ii) the
function will return a bool, if the ts variable was filled. Thus, in the RX
path if the function returns false, you do getnstimeofday() as fallback, and
in the TX path nothing needs to be done in __packet_set_timestamp() if the
skb
was not timestamped.
That said, for the software timestamps, the caller would not need to do more
as with your current patch, for the hardware part, it's then the same afaik
as in the RX_RING.
Just to give some pseudo code (instead of the bool, we could also use an
enum
that tells which timestamp was found, but more on that further below):
static bool tpacket_get_timestamp(struct sk_buff *skb, struct timespec *ts,
unsigned int flags)
{
struct skb_shared_hwtstamps *shhwtstamps = skb_hwtstamps(skb);
if (shhwtstamps) {
if ((flags & SOF_TIMESTAMPING_SYS_HARDWARE) &&
ktime_to_timespec_cond(shhwtstamps->syststamp, ts))
return true;
if ((flags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
ktime_to_timespec_cond(shhwtstamps->hwtstamp, ts))
return true;
}
if (ktime_to_timespec_cond(skb->tstamp, ts))
return true;
return false;
}
In tpacket_rcv():
...
if (!tpacket_get_timestamp(skb, &ts, po->tp_tstamp))
getnstimeofday(ts);
...
In __packet_set_timestamp():
...
if (!tpacket_get_timestamp(skb, &ts, po->tp_tstamp))
return;
...
quoted
quoted
Also the documentation [1] should receive a status update on
this
feature, otherwise only Paul will use this feature and nobody else.
Okay. I'll update the documentation. Speaking of which, it would be
great if someone could proofread the packet socket manpage update
patch (http://patchwork.ozlabs.org/patch/228709/)
Ok, I will have a look.
Saw your ack. Thanks, Daniel!
quoted
quoted
Not an expert in timestamping, but why can't we also allow hw timestamps
but
have
to use sw only?
To avoid getting into the same situation on tx that Richard pointed out
about rx: the ring can only communicate one timestamp and cannot
communicate which one it is. Better to be consistent in that case, I
think. I'm open to discussing this, though.
Ok, I agree that it would be very useful to also state what timestamp is
being reported. Ideally, without introducing yet another tpacket version.
The only thing I can currently think of could be to use the tp_status bit
for that, then used by both RX_RING and TX_RING, e.g. ...
TP_STATUS_TS_SYS_HARDWARE (1 << 30)
TP_STATUS_TS_RAW_HARDWARE (1 << 31)
... although it would be a bit of waste to use 2 bits for that. If none of
them is set, it indicates that the timestamp is in software-only. This we
could do, if you don't have a better idea.
I like that a lot. Visibility would certainly improve the state on
rx-ring side. One issue with interleaving various kinds of timestamps,
even when clearly labeled in tp_status, is that applications likely
cannot use timestamp series from different sources, as these sequences
are incomparable. Tx has the advantage that time sources can be chosen
per socket independent of all other sockets. The only time when
multiple sources come into play is when a socket has hw timestamps
selected, but the hardware failed to create a tstamp for some
device-specific reason. In that case, generating the sw timestamp may
or may not be helpful to the application. When at least it is clearly
labeled in tp_status, worst case the application will simply ignore
it. If the application only selects software timestamps, the mechanism
works just like what I proposed: always communicate the software
timestamp and do not modify tp_status. So, I'd say this is a clear
win. Since it is a strict superset, how about I send the current patch
as is (with extra documentation) and you submit the hw tstamp support
+ labels separately?
``Better to be consistent in that case'' would not be the current situation
with the patch, right? I mean, for RX_RING, we report one of those 3, for
TX_RING only the sw timestamp. So on one hand we might be consistent, but on
the other hand we're not. ;-) With the above proposal, we could tackle this
and stay consistent then. If you want, I could do that after your revised
patch gets accepted (with hw ts support). Let me know what you think of
that.
From: Daniel Borkmann <hidden> Date: 2013-04-21 18:15:10
On 04/21/2013 06:42 PM, Willem de Bruijn wrote:
win. Since it is a strict superset, how about I send the current patch
as is (with extra documentation) and you submit the hw tstamp support
+ labels separately?
From: Paul Chavent <hidden> Date: 2013-04-22 08:24:11
On 04/21/2013 06:42 PM, Willem de Bruijn wrote:
Tx has the advantage that time sources can be chosen
per socket independent of all other sockets
Sorry if my question is trivial.
I understand that when we require rx timestamping, we need to ask to the
device to timestamp all incoming packets since we don't know the path of
the packet in advance.
For tx timestamping, you seems to say that the request to timestamp the
packet is contained in the skbuff and is done on a per packet basis ?
From: Daniel Borkmann <hidden> Date: 2013-04-22 10:25:54
On 04/22/2013 10:19 AM, Paul Chavent wrote:
On 04/21/2013 06:42 PM, Willem de Bruijn wrote:
quoted
Tx has the advantage that time sources can be chosen
per socket independent of all other sockets
Sorry if my question is trivial.
I understand that when we require rx timestamping, we need to ask to the device to timestamp all incoming packets since we don't know the path of the packet in advance.
For tx timestamping, you seems to say that the request to timestamp the packet is contained in the skbuff and is done on a per packet basis ?
It's all in: Documentation/networking/timestamping.txt
From: Willem de Bruijn <willemb@google.com> Date: 2013-04-22 14:23:55
On Mon, Apr 22, 2013 at 4:19 AM, Paul Chavent [off-list ref] wrote:
On 04/21/2013 06:42 PM, Willem de Bruijn wrote:
quoted
Tx has the advantage that time sources can be chosen
per socket independent of all other sockets
Sorry if my question is trivial.
I understand that when we require rx timestamping, we need to ask to the
device to timestamp all incoming packets since we don't know the path of the
packet in advance.
For tx timestamping, you seems to say that the request to timestamp the
packet is contained in the skbuff and is done on a per packet basis ?
Yes. More precisely, this is set using a socket option and applies to
all packets on that socket sent since the last update of the socket
option.