* [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets @ 2007-08-08 11:50 Unai Uribarri 0 siblings, 0 replies; 10+ messages in thread From: Unai Uribarri @ 2007-08-08 11:50 UTC (permalink / raw) To: netdev; +Cc: linux-kernel Hello folks, I've discovered two strange behaviours (bugs?) about timestamp generation: 1. If a program opens an AF_PACKET socket and setup a reception ring with setsockopt(sock, SOL_PACKET, PACKET_RX_RING), timestamps are automatically (re)enabled at the reception of every packet. 2. Setting SOL_SOCKET/SO_TIMESTAMP to 0 doesn't disables timestamp generation. Every skb continues begin timestamped until you close the socket that activated it. Timestamp generation is a heavy task that is consuming more than 50% of the CPU (using ACPI PM clock) and is currently the bottleneck in my packet capturing application. The attached patch removes the automatic timestamp activation, that only mmap'ed AF_PACKET sockets perform. I known it can break user applications, but I believe that it's the correct solution. I will be very pleased to receive any feedback. Signed-off-by: Unai Uribarri <unai.uribarri@optenet.com> --- diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 1322d62..a4f2da3 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -640,10 +640,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, struct packe h->tp_snaplen = snaplen; h->tp_mac = macoff; h->tp_net = netoff; - if (skb->tstamp.tv64 == 0) { - __net_timestamp(skb); - sock_enable_timestamp(sk); - } tv = ktime_to_timeval(skb->tstamp); h->tp_sec = tv.tv_sec; h->tp_usec = tv.tv_usec; ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets @ 2007-08-09 14:21 Unai Uribarri 2007-08-09 14:33 ` Evgeniy Polyakov 0 siblings, 1 reply; 10+ messages in thread From: Unai Uribarri @ 2007-08-09 14:21 UTC (permalink / raw) To: netdev; +Cc: linux-kernel Hello folks, I've discovered two strange behaviours (bugs?) about timestamp generation: 1. If a program opens an AF_PACKET socket and setup a reception ring with setsockopt(sock, SOL_PACKET, PACKET_RX_RING), timestamps are automatically (re)enabled at the reception of every packet. 2. Setting SOL_SOCKET/SO_TIMESTAMP to 0 doesn't disables timestamp generation. Every skb continues begin timestamped until you close the socket that activated it. Timestamp generation is a heavy task that is consuming more than 50% of the CPU (using ACPI PM clock) and is currently the bottleneck in my packet capturing application. The attached patch removes the automatic timestamp activation, that only mmap'ed AF_PACKET sockets perform. I known it can break user applications, but I believe that it's the correct solution. I will be very pleased to receive any feedback. Signed-off-by: Unai Uribarri <unai.uribarri@optenet.com> --- diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 1322d62..a4f2da3 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -640,10 +640,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, struct packe h->tp_snaplen = snaplen; h->tp_mac = macoff; h->tp_net = netoff; - if (skb->tstamp.tv64 == 0) { - __net_timestamp(skb); - sock_enable_timestamp(sk); - } tv = ktime_to_timeval(skb->tstamp); h->tp_sec = tv.tv_sec; h->tp_usec = tv.tv_usec; ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-09 14:21 Unai Uribarri @ 2007-08-09 14:33 ` Evgeniy Polyakov 2007-08-09 18:13 ` Unai Uribarri 0 siblings, 1 reply; 10+ messages in thread From: Evgeniy Polyakov @ 2007-08-09 14:33 UTC (permalink / raw) To: Unai Uribarri; +Cc: netdev, linux-kernel On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > The attached patch removes the automatic timestamp activation, that > only mmap'ed AF_PACKET sockets perform. I known it can break user > applications, but I believe that it's the correct solution. How tcpdump with mmap libpcap will work with it? -- Evgeniy Polyakov ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-09 14:33 ` Evgeniy Polyakov @ 2007-08-09 18:13 ` Unai Uribarri 2007-08-09 18:18 ` Evgeniy Polyakov 0 siblings, 1 reply; 10+ messages in thread From: Unai Uribarri @ 2007-08-09 18:13 UTC (permalink / raw) To: Evgeniy Polyakov; +Cc: netdev, linux-kernel On jue, 2007-08-09 at 18:33 +0400, Evgeniy Polyakov wrote: > On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > The attached patch removes the automatic timestamp activation, that > > only mmap'ed AF_PACKET sockets perform. I known it can break user > > applications, but I believe that it's the correct solution. > > How tcpdump with mmap libpcap will work with it? In Linux, you can enable timestamps on any socket executing: int val = 1; setsockopt(sock, SOL_SOCKET, SO_TIMESTAMP, &val, sizeof(val)); PD: Current release of tcpdump doesn't mmap the reception ring and timestamps packets at user space with gettimeofday. It isn't the best performing alternative, but it's portable. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-09 18:13 ` Unai Uribarri @ 2007-08-09 18:18 ` Evgeniy Polyakov 2007-08-09 18:44 ` Unai Uribarri 2007-08-09 18:50 ` Unai Uribarri 0 siblings, 2 replies; 10+ messages in thread From: Evgeniy Polyakov @ 2007-08-09 18:18 UTC (permalink / raw) To: Unai Uribarri; +Cc: netdev, linux-kernel On Thu, Aug 09, 2007 at 08:13:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > On jue, 2007-08-09 at 18:33 +0400, Evgeniy Polyakov wrote: > > On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > > The attached patch removes the automatic timestamp activation, that > > > only mmap'ed AF_PACKET sockets perform. I known it can break user > > > applications, but I believe that it's the correct solution. > > > > How tcpdump with mmap libpcap will work with it? > > In Linux, you can enable timestamps on any socket executing: > > int val = 1; > setsockopt(sock, SOL_SOCKET, SO_TIMESTAMP, &val, sizeof(val)); > > PD: Current release of tcpdump doesn't mmap the reception ring and > timestamps packets at user space with gettimeofday. It isn't the best > performing alternative, but it's portable. IIRC, there was/is a libpcap which worked with mapped sockets, mybe not official though. Any application which depened on having timestamps with packets will not work now. Why not to implement an absolutely_turn_off_timestamps option instead of breaking compatibility? -- Evgeniy Polyakov ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-09 18:18 ` Evgeniy Polyakov @ 2007-08-09 18:44 ` Unai Uribarri 2007-08-10 8:34 ` Evgeniy Polyakov 2007-08-09 18:50 ` Unai Uribarri 1 sibling, 1 reply; 10+ messages in thread From: Unai Uribarri @ 2007-08-09 18:44 UTC (permalink / raw) To: Evgeniy Polyakov; +Cc: netdev, linux-kernel There is another option: 1. Move timestampt activation to packet_set_ring(), so it's activated only once at setup instead of every time a packet arrives. 2. Fix sock_setsockopt() so setting SO_TIMESTAMP to 0 effectively disables timestamp. My first patch set did that. I sent that patches in mime multipart format and they were removed from the list archives. I can resent them in plain text if needed. On jue, 2007-08-09 at 22:18 +0400, Evgeniy Polyakov wrote: > On Thu, Aug 09, 2007 at 08:13:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > On jue, 2007-08-09 at 18:33 +0400, Evgeniy Polyakov wrote: > > > On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > > > The attached patch removes the automatic timestamp activation, that > > > > only mmap'ed AF_PACKET sockets perform. I known it can break user > > > > applications, but I believe that it's the correct solution. > > > > > > How tcpdump with mmap libpcap will work with it? > > > > In Linux, you can enable timestamps on any socket executing: > > > > int val = 1; > > setsockopt(sock, SOL_SOCKET, SO_TIMESTAMP, &val, sizeof(val)); > > > > PD: Current release of tcpdump doesn't mmap the reception ring and > > timestamps packets at user space with gettimeofday. It isn't the best > > performing alternative, but it's portable. > > IIRC, there was/is a libpcap which worked with mapped sockets, mybe not > official though. Any application which depened on having timestamps with > packets will not work now. Why not to implement an > absolutely_turn_off_timestamps option instead of breaking compatibility? > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-09 18:44 ` Unai Uribarri @ 2007-08-10 8:34 ` Evgeniy Polyakov 2007-08-10 11:55 ` Unai Uribarri 0 siblings, 1 reply; 10+ messages in thread From: Evgeniy Polyakov @ 2007-08-10 8:34 UTC (permalink / raw) To: Unai Uribarri; +Cc: netdev, linux-kernel Hi Unai. On Thu, Aug 09, 2007 at 08:44:21PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > There is another option: > > 1. Move timestampt activation to packet_set_ring(), so it's activated > only once at setup instead of every time a packet arrives. Does this break existing systems which expects timestamp be turned on always if there are packet sockets. > 2. Fix sock_setsockopt() so setting SO_TIMESTAMP to 0 effectively > disables timestamp. This breaks compatibility. Add new socket option, which will really disable it and do all your logic, but not breaking existing applications. -- Evgeniy Polyakov ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-10 8:34 ` Evgeniy Polyakov @ 2007-08-10 11:55 ` Unai Uribarri 2007-08-10 12:14 ` Evgeniy Polyakov 0 siblings, 1 reply; 10+ messages in thread From: Unai Uribarri @ 2007-08-10 11:55 UTC (permalink / raw) To: Evgeniy Polyakov; +Cc: netdev, linux-kernel On vie, 2007-08-10 at 12:34 +0400, Evgeniy Polyakov wrote: > Hi Unai. > > On Thu, Aug 09, 2007 at 08:44:21PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > There is another option: > > > > 1. Move timestampt activation to packet_set_ring(), so it's activated > > only once at setup instead of every time a packet arrives. > > Does this break existing systems which expects timestamp be turned on > always if there are packet sockets. > Well, current behaviour is that all packets get always timestamped if the socket has a reception ring. We are just activating it a bit sooner at the setsockopt(SOL_PACKET, PACKET_RX_RING) call instead of waiting until the reception of the first packet. And current applications can't disable it if we use a new socket option. So I can see how an application can break. > > 2. Fix sock_setsockopt() so setting SO_TIMESTAMP to 0 effectively > > disables timestamp. > > This breaks compatibility. Add new socket option, which will really > disable it and do all your logic, but not breaking existing > applications. > Is SO_TIMESTAMP2 a valid name? I can't imagine how to call it. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-10 11:55 ` Unai Uribarri @ 2007-08-10 12:14 ` Evgeniy Polyakov 0 siblings, 0 replies; 10+ messages in thread From: Evgeniy Polyakov @ 2007-08-10 12:14 UTC (permalink / raw) To: Unai Uribarri; +Cc: netdev, linux-kernel On Fri, Aug 10, 2007 at 01:55:07PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > This breaks compatibility. Add new socket option, which will really > > disable it and do all your logic, but not breaking existing > > applications. > > > > Is SO_TIMESTAMP2 a valid name? I can't imagine how to call it. :) what about name, which really shows what option does? -- Evgeniy Polyakov ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets 2007-08-09 18:18 ` Evgeniy Polyakov 2007-08-09 18:44 ` Unai Uribarri @ 2007-08-09 18:50 ` Unai Uribarri 1 sibling, 0 replies; 10+ messages in thread From: Unai Uribarri @ 2007-08-09 18:50 UTC (permalink / raw) To: Evgeniy Polyakov; +Cc: netdev, linux-kernel Do not enable timestamps automatically on mmap'ed AF_PACKET sockets. --- commit d1d6e6bf196e31b6306fd0fef95f4190983c8a86 tree 22637506c0aafeabfbe05faf5352d0358c4d9460 parent 6a302358d87fedaf7bda12b8e909265ebf1ce674 author Unai Uribarri <unai.uribarri@optenet.com> Tue, 31 Jul 2007 20:38:42 +0200 committer Unai Uribarri <unai.uribarri@optenet.com> Tue, 31 Jul 2007 20:38:42 +0200 net/packet/af_packet.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 1322d62..a4f2da3 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -640,10 +640,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, struct packe h->tp_snaplen = snaplen; h->tp_mac = macoff; h->tp_net = netoff; - if (skb->tstamp.tv64 == 0) { - __net_timestamp(skb); - sock_enable_timestamp(sk); - } tv = ktime_to_timeval(skb->tstamp); h->tp_sec = tv.tv_sec; h->tp_usec = tv.tv_usec; !-------------------------------------------------------------flip- Effectively disable timestamping when requested by SO_TIMESTAMP --- commit 1fdf6bb534dfbc6e9bdf8958620b05d5334b15eb tree ec4b577c1704f178f0f7c5d8d69af41454fc8f14 parent d1d6e6bf196e31b6306fd0fef95f4190983c8a86 author Unai Uribarri <unai.uribarri@optenet.com> Tue, 31 Jul 2007 20:43:00 +0200 committer Unai Uribarri <unai.uribarri@optenet.com> Tue, 31 Jul 2007 20:43:00 +0200 net/core/sock.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/net/core/sock.c b/net/core/sock.c index cfed7d4..3af2322 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -561,6 +561,7 @@ set_rcvbuf: } else { sock_reset_flag(sk, SOCK_RCVTSTAMP); sock_reset_flag(sk, SOCK_RCVTSTAMPNS); + sock_disable_timestamp(sk); } break; !-------------------------------------------------------------flip- Automatically enable timestamping on mmap'ed AF_PACKET sockets. --- commit 4564f367ff054bd8837cc6cb1cfb9a927c57054a tree 3475d56cc7ea74052677c944ec0a6bf6e9f4817c parent 1fdf6bb534dfbc6e9bdf8958620b05d5334b15eb author Unai Uribarri <unai.uribarri@optenet.com> Tue, 31 Jul 2007 20:43:59 +0200 committer Unai Uribarri <unai.uribarri@optenet.com> Tue, 31 Jul 2007 20:43:59 +0200 net/packet/af_packet.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index a4f2da3..5179daf 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1750,6 +1750,7 @@ static int packet_set_ring(struct sock *sk, struct tpacket_req *req, int closing po->pg_vec_pages = req->tp_block_size/PAGE_SIZE; po->prot_hook.func = po->pg_vec ? tpacket_rcv : packet_rcv; + sock_enable_timestamp(sk); skb_queue_purge(&sk->sk_receive_queue); #undef XC if (atomic_read(&po->mapped)) !-------------------------------------------------------------flip- On jue, 2007-08-09 at 22:18 +0400, Evgeniy Polyakov wrote: > On Thu, Aug 09, 2007 at 08:13:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > On jue, 2007-08-09 at 18:33 +0400, Evgeniy Polyakov wrote: > > > On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote: > > > > The attached patch removes the automatic timestamp activation, that > > > > only mmap'ed AF_PACKET sockets perform. I known it can break user > > > > applications, but I believe that it's the correct solution. > > > > > > How tcpdump with mmap libpcap will work with it? > > > > In Linux, you can enable timestamps on any socket executing: > > > > int val = 1; > > setsockopt(sock, SOL_SOCKET, SO_TIMESTAMP, &val, sizeof(val)); > > > > PD: Current release of tcpdump doesn't mmap the reception ring and > > timestamps packets at user space with gettimeofday. It isn't the best > > performing alternative, but it's portable. > > IIRC, there was/is a libpcap which worked with mapped sockets, mybe not > official though. Any application which depened on having timestamps with > packets will not work now. Why not to implement an > absolutely_turn_off_timestamps option instead of breaking compatibility? > ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-08-10 12:14 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-08-08 11:50 [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets Unai Uribarri -- strict thread matches above, loose matches on Subject: below -- 2007-08-09 14:21 Unai Uribarri 2007-08-09 14:33 ` Evgeniy Polyakov 2007-08-09 18:13 ` Unai Uribarri 2007-08-09 18:18 ` Evgeniy Polyakov 2007-08-09 18:44 ` Unai Uribarri 2007-08-10 8:34 ` Evgeniy Polyakov 2007-08-10 11:55 ` Unai Uribarri 2007-08-10 12:14 ` Evgeniy Polyakov 2007-08-09 18:50 ` Unai Uribarri
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).