TPACKET_V3 timeout bug?

TPACKET_V3 timeout bug?

[Andrew Lunn]

Hi Folks

I'm running this simple program using libpcap:

#include <stdio.h>
#include <stdint.h>
#include <pcap/pcap.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
        struct pcap_pkthdr *hdr;
        const uint8_t *data;
        pcap_t *handle;
        int ret;


        handle = pcap_open_live("lan3", 65535, 1, 1000, NULL);
        if (handle == NULL)
                exit(EXIT_FAILURE);

        while (1) {
                ret = pcap_next_ex(handle, &hdr, &data);
                printf("ret: %d\n", ret);
        }
}

The man page says:

       pcap_t *pcap_open_live(const char *device, int snaplen,
               int promisc, int to_ms, char *errbuf);

DESCRIPTION

       pcap_open_live() is used to obtain a packet capture handle to
       look at packets on the network.  device is a string that
       specifies the network device to open; on Linux systems with 2.2
       or later kernels, a device argument of "any" or NULL can be
       used to capture packets from all interfaces.

       snaplen specifies the snapshot length to be set on the handle.

       promisc specifies if the interface is to be put into promiscuous mode.

       to_ms specifies the read timeout in milliseconds.

so i'm passing to_ms for 1000ms.

and the man page for pcap_next_ex() says:

RETURN VALUE

       pcap_next_ex() returns 1 if the packet was read without
       problems, 0 if packets are being read from a live capture and
       the timeout expired

In my case, lan3 is up and idle, there are no packets flying around to
be captured. So i would expect pcap_next_ex() to exit once a second,
with a return value of 0. But it is not, it blocks and stays blocked.

strace shows:

socket(AF_PACKET, SOCK_RAW, 768)        = 3
ioctl(3, SIOCGIFINDEX, {ifr_name="lo", }) = 0
ioctl(3, SIOCGIFHWADDR, {ifr_name="lan3", ifr_hwaddr=94:10:3e:80:bc:f3}) = 0
stat64("/sys/class/net/lan3/wireless", 0xbe9443a0) = -1 ENOENT (No such file or directory)
ioctl(3, SIOCBONDINFOQUERY, 0xbe944304) = -1 EOPNOTSUPP (Operation not supported)
ioctl(3, SIOCGIWNAME, 0xbe944354)       = -1 EINVAL (Invalid argument)
ioctl(3, SIOCGIFINDEX, {ifr_name="lan3", }) = 0
bind(3, {sa_family=AF_PACKET, sll_protocol=htons(ETH_P_ALL), sll_ifindex=if_nametoindex("lan3"), sll_hatype=ARPHRD_NETROM, sll_pkttype=PACKET_HOST, sll_halen=0}, 20) = 0
getsockopt(3, SOL_SOCKET, SO_ERROR, [0], [4]) = 0
setsockopt(3, SOL_PACKET, PACKET_ADD_MEMBERSHIP, {mr_ifindex=5, mr_type=PACKET_MR_PROMISC, mr_alen=0, mr_address=}, 16) = 0
setsockopt(3, SOL_PACKET, PACKET_AUXDATA, [1], 4) = 0
getsockopt(3, SOL_SOCKET, SO_BPF_EXTENSIONS, [64], [4]) = 0
getsockopt(3, SOL_PACKET, PACKET_HDRLEN, [48], [4]) = 0
setsockopt(3, SOL_PACKET, PACKET_VERSION, [2], 4) = 0
setsockopt(3, SOL_PACKET, PACKET_RESERVE, [4], 4) = 0
setsockopt(3, SOL_PACKET, PACKET_RX_RING, 0xbe9445c0, 28) = 0
mmap2(NULL, 2097152, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = 0xb6c93000
uname({sysname="Linux", nodename="wrt1900ac", ...}) = 0
poll([{fd=3, events=POLLIN}], 1, -1

Looking at the libpcap source, the 1000ms timeout is being used as
part of the setsockopt(3, SOL_PACKET, PACKET_RX_RING, 0xbe9445c0, 28)
call, req.tp_retire_blk_tov is set to the timeoutval.

And libpcap, when determining the timeout value to pass to poll has
the comment:

	 * For TPACKET_V3, the timeout is handled by the kernel,
	 * so block forever; that way, we don't get extra timeouts.

So it is expecting poll() to exit from a blocking call when
req.tp_retire_blk_tov expires.

So i think there is a bug, or a wrong understanding of the kernel API.
Any suggestions which?

Thanks
	Andrew

Re: TPACKET_V3 timeout bug?

[Sowmini Varadhan]

On (04/15/17 21:40), Andrew Lunn wrote:
> 
> In my case, lan3 is up and idle, there are no packets flying around to
> be captured. So i would expect pcap_next_ex() to exit once a second,
> with a return value of 0. But it is not, it blocks and stays blocked.
   :
> Looking at the libpcap source, the 1000ms timeout is being used as
> part of the setsockopt(3, SOL_PACKET, PACKET_RX_RING, 0xbe9445c0, 28)
> call, req.tp_retire_blk_tov is set to the timeoutval.

right, aiui, the retire_blk_tov will only kick in if we have at
least one frame in a block, but the block is not filled up yet,
before the req.tp_retire_blk_tov (1s in your case) expires.

If there are 0 frames pending, we should not be waking up the app,
so everything seems to be behaving as it should?

--Sowmini

Re: TPACKET_V3 timeout bug?

[Andrew Lunn]

On Sat, Apr 15, 2017 at 06:45:36PM -0400, Sowmini Varadhan wrote:
> On (04/15/17 21:40), Andrew Lunn wrote:
> > 
> > In my case, lan3 is up and idle, there are no packets flying around to
> > be captured. So i would expect pcap_next_ex() to exit once a second,
> > with a return value of 0. But it is not, it blocks and stays blocked.
>    :
> > Looking at the libpcap source, the 1000ms timeout is being used as
> > part of the setsockopt(3, SOL_PACKET, PACKET_RX_RING, 0xbe9445c0, 28)
> > call, req.tp_retire_blk_tov is set to the timeoutval.
> 
> right, aiui, the retire_blk_tov will only kick in if we have at
> least one frame in a block, but the block is not filled up yet,
> before the req.tp_retire_blk_tov (1s in your case) expires.
> 
> If there are 0 frames pending, we should not be waking up the app,
> so everything seems to be behaving as it should?

Hi Sowmini 

Humm, i can see the logic of that, it puts an upper bound on the
latency for delivering a frame to user space, but does not wake user
space when there is nothing in the queue.

Yet i'm debugging an application which expects a timeout even when
there are 0 packets. The Ostinator drone. It is a multi thread
process, with a thread performing capture, and another thread doing
control stuff. When the control thread wants to stop the capturing, it
is setting a variable.  The next time the capture thread comes out of
pcap_next_en() it checks the variable and close the capture and the
thread exists. But if there is no network traffic, it never
exists. This scheme has worked before, but suddenly stopped when i
upgraded something. What i cannot say is if that is libpcap, or a
kernel, since i upgraded both at the same time.

But it does seem like a regression somewhere.

Looking at libpcap, it does seem to expect a timeout to happen even
when there are 0 packets available. Has there been a kernel change
with respect to this behaviour?

  Andrew

Re: TPACKET_V3 timeout bug?

[Guy Harris]

On Apr 15, 2017, at 4:44 PM, Andrew Lunn <andrew@lunn.ch> wrote:

> Yet i'm debugging an application which expects a timeout even when
> there are 0 packets.

Well, you've already found a bug - it expects a timeout where there are no packets.

To quote the pcap man page (this is the latest version, which calls it the "packet buffer timeout" rather than the "read timeout"):

       packet buffer timeout
              If,  when  capturing,  packets  are  delivered  as  soon as they
              arrive, the application capturing the packets will be  woken  up
              for  each  packet  as  it arrives, and might have to make one or
              more calls to the operating system to fetch each packet.

              If, instead, packets are not delivered as soon as  they  arrive,
              but  are  delivered after a short delay (called a "packet buffer
              timeout"), more than one packet can be  accumulated  before  the
              packets are delivered, so that a single wakeup would be done for
              multiple packets, and each set of calls made  to  the  operating
              system  would  supply  multiple  packets,  rather  than a single
              packet.  This reduces the per-packet CPU overhead if packets are
              arriving  at  a  high rate, increasing the number of packets per
              second that can be captured.

              The packet buffer timeout is required  so  that  an  application
              won't  wait for the operating system's capture buffer to fill up
              before packets are delivered; if packets  are  arriving  slowly,
              that wait could take an arbitrarily long period of time.

              Not  all platforms support a packet buffer timeout; on platforms
              that don't, the packet buffer timeout is ignored.  A zero  value
              for the timeout, on platforms that support a packet buffer time-
              out, will cause a read to wait forever to allow  enough  packets
              to arrive, with no timeout.

              NOTE:  the  packet  buffer timeout cannot be used to cause calls
              that read packets to return within a  limited  period  of  time,
              because, on some platforms, the packet buffer timeout isn't sup-
              ported, and, on other platforms, the timer doesn't  start  until
              at  least one packet arrives.  This means that the packet buffer
              timeout should NOT be  used,  for  example,  in  an  interactive
              application  to  allow  the  packet capture loop to ``poll'' for
              user input periodically, as there's no  guarantee  that  a  call
              reading packets will return after the timeout expires even if no
              packets have arrived.

              The packet buffer timeout is set with pcap_set_timeout().

Note especially the next-to-last paragraph - which was put in there long before TPACKET_V3, to cover, for example, Solaris.

The purpose of the timeout is to make sure packets don't stay stuck in the kernel buffer for an indefinite period of time (if they're not arriving at a sufficient rate to fill up the buffer in a reasonably-short period of time); it's *not* to make sure the application doesn't remain blocked for an indefinite period of time.

Re: TPACKET_V3 timeout bug?

[Andrew Lunn]

On Sat, Apr 15, 2017 at 06:45:36PM -0400, Sowmini Varadhan wrote:
> On (04/15/17 21:40), Andrew Lunn wrote:
> > 
> > In my case, lan3 is up and idle, there are no packets flying around to
> > be captured. So i would expect pcap_next_ex() to exit once a second,
> > with a return value of 0. But it is not, it blocks and stays blocked.
>    :
> > Looking at the libpcap source, the 1000ms timeout is being used as
> > part of the setsockopt(3, SOL_PACKET, PACKET_RX_RING, 0xbe9445c0, 28)
> > call, req.tp_retire_blk_tov is set to the timeoutval.
> 
> right, aiui, the retire_blk_tov will only kick in if we have at
> least one frame in a block, but the block is not filled up yet,
> before the req.tp_retire_blk_tov (1s in your case) expires.
> 
> If there are 0 frames pending, we should not be waking up the app,
> so everything seems to be behaving as it should?

Hi Guy

You wrote the TPACKET3 set_poll_timeout() handling. Please could you
have a look at the thread:

https://www.mail-archive.com/netdev@vger.kernel.org/msg163532.html

Do you think this is a kernel problem, libpcap problem, or an
application problem?

Thanks
	Andrew

Re: TPACKET_V3 timeout bug?

[Guy Harris]

On Apr 15, 2017, at 7:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:

> Do you think this is a kernel problem, libpcap problem, or an
> application problem?

An application problem.  See my response on netdev; the timeout (which is provided by the kernel's capture mechanism, in most cases) is to make sure packets don't stay stuck in the kernel's packet buffer for an indefinite period of time, it's *not* to make sure a thread capturing packets doesn't stay blocked for an indefinite period of time.  Whether the timer goes off even if no packets have arrived is platform-dependent; code capturing packets should neither depend on it doing so or on it not doing so.

Re: TPACKET_V3 timeout bug?

[chetan loke]

On Sat, Apr 15, 2017 at 7:41 PM, Guy Harris <guy@alum.mit.edu> wrote:
> On Apr 15, 2017, at 7:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>
>> Do you think this is a kernel problem, libpcap problem, or an
>> application problem?
>

Its clearly a kernel regression.

If you look at if_packet.h, I have explicitly called out all the cases
for the return/status codes. When I first merged the functionality in
3.11(or 3.12 I think) I had the logic in place to retire the block
with or without packets in it. I think there was one case where we
wouldn't wake up userspace. Someone checked in a fix for that. Now I
am not sure the regression happened as part of that bug fix or
sometime later. If you diff 3.12 against the latest you will find the
regression. Look for prb_retire_rx_blk_timer_expired().


> An application problem.  See my response on netdev; the timeout (which is provided by the kernel's capture mechanism, in most cases) is to make sure packets don't stay stuck in the kernel's packet buffer for an indefinite period of time, it's *not* to make sure a thread capturing packets doesn't stay blocked for an indefinite period of time.  Whether the timer goes off even if no

I cannot speak on behalf of user-space wrappers developed around
tpacket_v3 but the intention(from the kernel POV) of the block_timer
*is* to unblock the capture/user process/thread so that it does NOT
stay blocked for an indefinite period of time. The header explicitly
specifies that contract.

I'm tied up for the next 3 weeks. However I can work on it after
that(unless someone else has cycles for it now).

Chetan

Re: TPACKET_V3 timeout bug?

[chetan loke]

On Tue, May 2, 2017 at 8:04 AM, chetan loke <loke.chetan@gmail.com> wrote:
> On Sat, Apr 15, 2017 at 7:41 PM, Guy Harris <guy@alum.mit.edu> wrote:
>> On Apr 15, 2017, at 7:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>>
>>> Do you think this is a kernel problem, libpcap problem, or an
>>> application problem?
>>
>
> Its clearly a kernel regression.
>

Commit that caused it:

https://github.com/torvalds/linux/commit/41a50d621a321b4c15273cc1b5ed41437f4acdfb

Reverting that change is what we need.

When monitoring aggregated links (example: request going out on one
link and response coming in on another) you have to know when to start
mining packets for time-interval[s] to report anomalies etc. And the
block-retirement was designed such that the kernel would fill those
values for the app in the ts_first[/last]_pkt.

We have already amortized the cost by doing a bulk wakeup. And if
user-space holds on to the block even when it is empty (instead of
just using/copying the start/end time interval and releasing the
block) then its a bug in your app.

And if user-space is lagging behind then you will always run out of
buffer space. So I don't buy the entire commit argument of the patch.

Re: TPACKET_V3 timeout bug?

[Guy Harris]

On May 2, 2017, at 10:16 AM, chetan loke <loke.chetan@gmail.com> wrote:

> Commit that caused it:
> 
> https://github.com/torvalds/linux/commit/41a50d621a321b4c15273cc1b5ed41437f4acdfb
> 
> Reverting that change is what we need.

As long as you do *not* revert

	https://github.com/torvalds/linux/commit/da413eec729dae5dcb150e2eb34c5e7e5e4e1b49

"packet: Fixed TPACKET V3 to signal poll when block is closed rather than every packet

Make TPACKET_V3 signal poll when block is closed rather than for every
packet. Side effect is that poll will be signaled when block retire
timer expires which didn't previously happen. Issue was visible when
sending packets at a very low frequency such that all blocks are retired
before packets are received by TPACKET_V3. This caused avoidable packet
loss. The fix ensures that the signal is sent when blocks are closed
which covers the normal path where the block is filled as well as the
path where the timer expires. The case where a block is filled without
moving to the next block (ie. all blocks are full) will still cause poll
to be signaled."

The behavior in the commit message exactly what libpcap (and probably at least some other users of TPACKET_V3) need.  libpcap doesn't care whether empty blocks are passed up (as indicated, "you get woken up when the packet buffer timeout expires" is *not* part of libpcap's contract, and code shouldn't depend on that), so, as long as you preserve the "signal poll when a block is closed, even if it's an empty block" behavior, libpcap will work fine.

Re: TPACKET_V3 timeout bug?

[Guy Harris]

On May 2, 2017, at 8:04 AM, chetan loke <loke.chetan@gmail.com> wrote:

> On Sat, Apr 15, 2017 at 7:41 PM, Guy Harris <guy@alum.mit.edu> wrote:
>> On Apr 15, 2017, at 7:10 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> 
>>> Do you think this is a kernel problem, libpcap problem, or an
>>> application problem?
>> 
> 
> Its clearly a kernel regression.
> 
> If you look at if_packet.h, I have explicitly called out all the cases
> for the return/status codes. When I first merged the functionality in
> 3.11(or 3.12 I think) I had the logic in place to retire the block
> with or without packets in it. I think there was one case where we
> wouldn't wake up userspace. Someone checked in a fix for that. Now I
> am not sure the regression happened as part of that bug fix or
> sometime later. If you diff 3.12 against the latest you will find the
> regression. Look for prb_retire_rx_blk_timer_expired().

Yes, there's a case where user space wasn't being woken up.

As I said in

	https://github.com/the-tcpdump-group/libpcap/issues/335#issuecomment-30280794

It appeared, at the time, that PF_PACKET sockets delivered a wakeup when a packet is put in a buffer block or dropped due to no buffer blocks being empty, but not when a buffer block is handed to userland.

This means that if the kernel's timer expires, and there are no packets in the current buffer block being filled by the kernel, that buffer block will be handed to userland, but userland won't be woken up to tell it to consume that block.

Thus, libpcap will consume that block only if either:

	* a packet is put in a buffer block, meaning it must pass the filter and there must be a current buffer block, belonging to the kernel, into which to put it;
	* a packet arrives and passes the filter, but there are no current buffer blocks belonging to the kernel, so it's dropped;
	* the poll() times out.

So, with a low packet acceptance rate (either because there isn't much network traffic or because there is but most of it is rejected by the packet filter), and with a poll() timeout of -1, meaning "block forever", 1) will happen infrequently, and 3) will never happen. With an in-kernel timeout rate significantly lower than the rate of packet acceptance, the timeout will often occur when there are no packets in the current buffer block, in which case the kernel will hand an empty buffer block to userland and not tell userland about it.

If that happens often enough in sequence to cause all buffer blocks to be handed to userland before any wakeups occur, the kernel now has no buffer blocks into which to put packets, and the next time a packet arrives, it will be dropped, and a wakeup will finally occur. libpcap will drain the ring, handing all buffer blocks to the kernel, but it won't have any packets to process!

So this is ultimately a problem with the TPACKET_V3 code in the kernel. I personally think that it should not deliver empty buffer blocks to userland, and that it also should not deliver a wakeup when a packet is accepted, and should deliver a wakeup whenever a buffer block is handed to userland. I'll report this to somebody and let them decide which of those changes should be done.

If you want to deliver empty buffer blocks to userland, that's fine, but make sure you wake up userland so that it can process those packets rather than leaving them there taking up space in the ring buffer.

And if you insist on delivering a wakeup when a packet is accepted - a wakeup that libpcap, at least, won't do anything with, as there's nothing useful for it to do with that wakeup - also make sure you deliver a wakeup when a buffer block is handed to userland, which is what libpcap cares about.

> I cannot speak on behalf of user-space wrappers developed around
> tpacket_v3 but the intention(from the kernel POV) of the block_timer
> *is* to unblock the capture/user process/thread so that it does NOT
> stay blocked for an indefinite period of time. The header explicitly
> specifies that contract.

That's not part of the contract for libpcap, as it's a question of what the underlying capture mechanism does, and we don't necessarily have any control over that; if a particular capture mechanism used by libpcap has that as part of its contract, that's OK, but libpcap-based applications shouldn't depend on it.

Re: TPACKET_V3 timeout bug?

[Guy Harris]

On May 2, 2017, at 10:54 AM, Guy Harris <guy@alum.mit.edu> wrote:

> Yes, there's a case where user space wasn't being woken up.

See also this thread from almost 3 years ago, beginning with

	http://marc.info/?l=linux-netdev&m=140633612828824&w=2

and this patch thread from almost 2 1/2 years ago:

	https://lkml.org/lkml/2014/12/18/466