Re: bufs lost in TCP connection establishment


Jukka Rissanen
 

Hi Rohit,

On Mon, 2016-09-12 at 10:43 +0000, Rohit Grover wrote:
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.
Your patch makes sense, just submit it to gerrit.


Cheers,
Jukka

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