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 <[email protected]>
---
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;
On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri ([email protected]) 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
On jue, 2007-08-09 at 18:33 +0400, Evgeniy Polyakov wrote:
> On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri ([email protected]) 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.
On Thu, Aug 09, 2007 at 08:13:54PM +0200, Unai Uribarri ([email protected]) 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 ([email protected]) 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
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 ([email protected]) 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 ([email protected]) 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?
>
Do not enable timestamps automatically on mmap'ed AF_PACKET sockets.
---
commit d1d6e6bf196e31b6306fd0fef95f4190983c8a86
tree 22637506c0aafeabfbe05faf5352d0358c4d9460
parent 6a302358d87fedaf7bda12b8e909265ebf1ce674
author Unai Uribarri <[email protected]> Tue, 31 Jul 2007 20:38:42 +0200
committer Unai Uribarri <[email protected]> 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 <[email protected]> Tue, 31 Jul 2007 20:43:00 +0200
committer Unai Uribarri <[email protected]> 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 <[email protected]> Tue, 31 Jul 2007 20:43:59 +0200
committer Unai Uribarri <[email protected]> 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 ([email protected]) 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 ([email protected]) 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?
>
Hi Unai.
On Thu, Aug 09, 2007 at 08:44:21PM +0200, Unai Uribarri ([email protected]) 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
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 ([email protected]) 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.
On Fri, Aug 10, 2007 at 01:55:07PM +0200, Unai Uribarri ([email protected]) 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