| Message ID | 1355326165-12277-1-git-send-email-paul.chavent@onera.fr |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
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
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
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
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
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
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
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
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
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
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;
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(+)