[lwip-devel] [bug #49914] lwip_sendmsg uses PBUF_REF pbufs

[David van Moolenbroek]

Follow-up Comment #5, bug #49914 (project lwip):

My 2c here, which probably just rehashes much of what's being discussed
elsewhere already, but with a concrete proposal..

Joel's case makes clear that copying anywhere except at the netif/driver layer
may lead to unnecessary copying, so it makes sense to perform those copies
only at that layer. At the same time, zero-copy implementations in that
netif/driver layer will want to eliminate as much copying as is safe. With
these two points combined, it is obvious that it should be *crystal clear* to
driver writers when a pbuf should be copied (if immediate transmission is not
possible) as opposed to just increasing its refcount. Simon pointed this out
in the other thread as well. In any case I don't think I need to emphasize how
difficult it is to debug cases where a wrong assumption is made.. :)

As such, I would propose adding a function like,

  /* pbuf2 == pbuf if referenced
   * pbuf2 != pbuf if copied
   * pbuf2 == NULL if out of memory */
  pbuf2 = netif_zerocopy_ref_or_copy(pbuf);

or perhaps simply

  if (netif_zerocopy_must_copy(pbuf)) { ..do the rest yourself..

The actual implementation of these functions could simply check if there is
any PBUF_REF pbuf in the chain, and if so, (instruct to) make a full copy of
the whole chain. Driver programmers then have an API to program against, and
the implementation can be changed later as needed.

I would argue that TCP is a slightly different case. Making copies of entire
TCP packets up front on the off chance that they will be retransmitted later
would be horribly expensive and pretty much negate the main benefits of
zero-copy at all. Right now, TCP does in fact check the reference count of
segments (right at the start of tcp_output_segment()) and simply does not
perform retransmission if it's greater than one. There should hopefully be no
other cases where already-transmitted packets are modified, but if the current
check is not adequate to prevent problems with zero-copy, I would consider
that a bug.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?49914>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/