Patchwork [RFC] net : add tx timestamp to packet mmap.

login
register
mail settings
Submitter Paul Chavent
Date Dec. 12, 2012, 3:29 p.m.
Message ID <1355326165-12277-1-git-send-email-paul.chavent@onera.fr>
Download mbox | patch
Permalink /patch/205567/
State RFC
Delegated to: David Miller
Headers show

Comments

Paul Chavent - Dec. 12, 2012, 3:29 p.m.
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 <paul.chavent@onera.fr>
---
 net/packet/af_packet.c | 4 ++++
 1 file changed, 4 insertions(+)
David Miller - Dec. 12, 2012, 7:23 p.m.
You're changing the code that handles sendmsg() and then wondering why
a recvmsg() call doesn't provide a timestamp.
--
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
Paul Chavent - Dec. 13, 2012, 7:13 a.m.
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.
>
--
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
Richard Cochran - Dec. 13, 2012, 1:29 p.m.
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() ...

> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index e639645..948748b 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1857,6 +1857,10 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
>  	void *data;
>  	int err;
>  
> +	err = sock_tx_timestamp(&po->sk, &skb_shinfo(skb)->tx_flags);

and this call is only setting some flags.

HTH,
Richard
--
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
Paul Chavent - Dec. 13, 2012, 4:13 p.m.
Hello.

On 12/13/2012 02:29 PM, Richard Cochran wrote:
> 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() ...
Yes. "Somebody" means "the hardware driver" after completing xmit. 
That's true ?

>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index e639645..948748b 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -1857,6 +1857,10 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
>>   	void *data;
>>   	int err;
>>
>> +	err = sock_tx_timestamp(&po->sk, &skb_shinfo(skb)->tx_flags);
 >
 > 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 ?

>
> 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
Richard Cochran - Dec. 13, 2012, 6:17 p.m.
On Thu, Dec 13, 2012 at 05:13:56PM +0100, Paul Chavent wrote:
> >
> >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
--
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
Paul Chavent - Dec. 14, 2012, 7:57 a.m.
On 12/13/2012 07:17 PM, Richard Cochran wrote:
> On Thu, Dec 13, 2012 at 05:13:56PM +0100, Paul Chavent wrote:
>>>
>>> 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).


>
>> 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.

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.
--
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
Paul Chavent - April 9, 2013, 10:42 a.m.
On 12/13/2012 07:17 PM, Richard Cochran wrote:
> On Thu, Dec 13, 2012 at 05:13:56PM +0100, Paul Chavent wrote:
>>>
>>> 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
>
Hi.

Would it be possible that the packet mmap maintainers give their opinion 
on this thread please ?

Regards.

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
Richard Cochran - April 9, 2013, 1:15 p.m.
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 <scott.a.mcmillan@intel.com>
 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
--
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
Willem de Bruijn - April 13, 2013, 6:33 p.m.
On Thu, Dec 13, 2012 at 11:13 AM, Paul Chavent <Paul.Chavent@onera.fr> wrote:
> Hello.
>
>
> On 12/13/2012 02:29 PM, Richard Cochran wrote:
>>
>> 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() ...
>
> Yes. "Somebody" means "the hardware driver" after completing xmit. That's
> true ?
>
>
>>
>>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>>> index e639645..948748b 100644
>>> --- a/net/packet/af_packet.c
>>> +++ b/net/packet/af_packet.c
>>> @@ -1857,6 +1857,10 @@ static int tpacket_fill_skb(struct packet_sock
>>> *po, struct sk_buff *skb,
>>>         void *data;
>>>         int err;
>>>
>>> +       err = sock_tx_timestamp(&po->sk, &skb_shinfo(skb)->tx_flags);
>
>>
>> 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 ?

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.

>>
>> 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
--
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

Patch

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index e639645..948748b 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1857,6 +1857,10 @@  static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
 	void *data;
 	int err;
 
+	err = sock_tx_timestamp(&po->sk, &skb_shinfo(skb)->tx_flags);
+	if (err < 0)
+		return err;
+
 	ph.raw = frame;
 
 	skb->protocol = proto;