Broken socket accounting about tempesta HOT 10 CLOSED

The problem analyzing is not enough. The warning

    WARNING: at net/core/stream.c:201 sk_stream_kill_queues+0x12b/0x160()

is produced by

   WARN_ON(sk->sk_forward_alloc)

So which exactly callback called by skb->destructor decrements the counter? Which callback is registered for skb->destructor when we process the skb in ss_tcp_process_data()?

Also why __kfree_skb() called anyway for the skb doesn't decrement the counter properly?

Please explain all the points.

from tempesta.

sk->sk_forward_alloc isn't decremented. It's incremented. Different callbacks may do that, such as sk_stream_rfree() or sock_rfree(). They may be assigned to skb->destructor by sk_stream_set_owner_r() or sk_set_owner_r(). From the accounting point of view it doesn't really matter as they do the same kind of a job. What does matter is a general principle of what happens and how that happens. According to it, we need to adjust accounting when we remove an skb from the socket's receive queue as that skb had been accounted for at the time it had been put there. We get a warning (not an error) if we don't do that. The system takes care of that by setting up an approproate skb destructor. In the regular course of things that happens when an skb is deleted as the data that it held has been consumed by an application.

Similar thing happens on the egress path.

The sk->sk_forward_alloc counter is for socket's own accounting. It has nothing to do with skb accounting. An skb is freed by calling __kfree_skb() when it's no longer needed. An skb destructor is called when the skb is freed, and the destructor makes all final touches that are required.

from tempesta.

The unclear thing is that __kfree_skb() calls skb->destructor() in skb_release_head_state() called by skb_release_all(). So why do we need to call skb->destructor() explicitly via skb_orphan() as in the patch?

from tempesta.

In the comment to the path it's said that in the regualar course of action socket accounting data is adjusted when an SKB is freed. As explained above, that happens as stream data is consumed by an application, and the data is no longer needed to hang around in the kernel.

That's not what happens in Tempesta though. In the Synchronous Sockets architecture we "consume" the stream data inside Tempesta by taking SKBs out of the receive queue of one socket and putting them into the send queue of another, totally different socket. __kfree_skb() is never called for the receive socket in this case, and so the destructor is not called as well. These SKBs are then "re-owned" by the egress socket while not being correctly "un-owned" from the ingress socket. That's where orphaning comes in, and that's why we need to call it. Otherwise we see a warning for broken accounting in the ingress socket.

from tempesta.

Ok, thanks. It seems we reassign destructor callback when pass the skb to other socket, this is normally done by skb_set_owner_{r,w}(). I'll investigate the issue more as part of overall Synchronous Sockets architecture review.

from tempesta.

This issue is somewhat connected with #56. If some SKBs are not released or released incorrectly, then socket accounting may break.

from tempesta.

It turned out that the kernel was giving accounting warnings on a socket that didn't belong to Tempesta. Issue #96 was discovered while researching this accounting problem. This problem is not reproduced after #96 was fixed.

from tempesta.

Recent Tempesta execution traces show that this issue is still pending, and this time it is happening with Tempesta sockets, which is proven by extra specific printouts added to the Linux kernel code.

This issue is actually caused by irregular freeing of SKBs that make up HTTP messages processed by Tempesta. Socket accounting is managed by calls to skb->destructor() when SKBs are freed. When an SKB is not released as it should be, that keeps bytes occoupied by that SKB accounted for by the socket, which, in turn, causes these kernel warnings when the socket is released.

This issue will be solved as soon as all SKBs are released properly, which is the essense of issue #56.

from tempesta.

Warning WARNING: at net/core/stream.c:201 sk_stream_kill_queues for current master isn't 100% reproducable due to earlier crashes on skb freeing. I see the warnings in 30-50% of runs.

from tempesta.

This issue no longer comes up as SKBs are freed correctly now (see #56).

from tempesta.