Re: bufs lost in TCP connection establishment


Rohit Grover
 

Hi all,

I've discovered that there is a potential for a refcount leak if net_send() isn't the only API generating TX buffers.

With the following patch, I'm able to avoid the ref-count leak for the SYN buffer during connection establishment.

<patch>
diff --git a/net/ip/contiki/ipv4/uip.c b/net/ip/contiki/ipv4/uip.c
index 7a1e311..a7b99c3 100644
--- a/net/ip/contiki/ipv4/uip.c
+++ b/net/ip/contiki/ipv4/uip.c
@@ -1916,9 +1916,8 @@ uip_process(struct net_buf **buf_out, uint8_t flag)
net_context_set_connection_status(ip_buf_context(uip_connr->buf),
EISCONN);

- /* Eventually the uip_connr->buf will be freed
- * by net_core.c:net_send()
- */
+ ip_buf_unref(uip_connr->buf);
+ uip_connr->buf = NULL;

tcp_cancel_retrans_timer(uip_connr);
</patch>

It turns out that net_send() is meant to release the refcount for the SYN buffer, but that assumes that the application uses net_send() for all outgoing buffers. In my application, I'm using net_context_tcp_init() to initiate connections; so I'm breaking the assumption around net_send().

With the above change, we remove the dependency on net_send being called. If there are no objections, I'll submit this as a pull-request.
Your feedback is very welcome.

rohit.

-----Original Message-----
> From: Jukka Rissanen [mailto:jukka.rissanen(a)linux.intel.com]
> Sent: 08 September 2016 07:14
> To: Rohit Grover; Paul Sokolovsky
> Cc: devel(a)lists.zephyrproject.org; Marcus Shawcroft
> Subject: Re: [devel] Re: bufs lost in TCP connection establishment
>
> Hi Rohit,
>
> On Wed, 2016-09-07 at 19:42 +0000, Rohit Grover wrote:
> > Jukka,
> >
> > I have used CONFIG_NETWORK_IP_STACK_DEBUG_NET_BUF. And it does
> tell me
> > that the buf involved in sending out the SYN packet doesn't get
> > released because of a non-zero refcount.
> >
> > Tomorrow I'll share some log output to prove my claim.
>
> I am not doubting you here, I am sure there is a buffer leak if you have seen
> it in the logs. I was just wondering if the log would help to identify the
> culprit.
>
> I am suspecting that the patch "net: revert tcpip_poll_tcp() to not require a
> data_buf" might be the reason for the leak.
>
> The tcpip_poll_tcp(struct uip_conn *conn) function was changed back to
> have
> conn->buf = ip_buf_ref(buf);
>
> And this ref is probably missing the corresponding unref(). It might be that
> this ref is not really needed. Can you try and remove it and check if it fixes
> the issue.
>
>
> Cheers,
> Jukka

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Join devel@lists.zephyrproject.org to automatically receive all group messages.