net/dhcpv4: Fix single byte buffer filling madness

[therealprof]

The DHCP client is adding an empty file and sname in a tremendously wasteful way:

static inline bool dhcpv4_add_sname(struct net_pkt *pkt)
{
        u8_t len = SIZE_OF_SNAME;

        while (len-- > 0) {
                if (!net_pkt_append_u8(pkt, 0)) {
                        return false;
                }
        }

        return true;
}

causing a boatload of net_buf_simple_add calls per outgoing DHCPv4 packet:

[net/dhcpv4] [DBG] dhcph4_manage_timers: (0x20006930): iface 0x2000dd20 state=selecting
[net/buf] [DBG] net_buf_alloc_len_debug: (0x20006930) net_pkt_get_reserve_data_debug():357: pool 0x2000dcf0 size 128 timeout -1
[net/buf] [DBG] net_buf_alloc_len_debug: (0x20006930) allocated buf 0x20009e68
[net/buf] [DBG] net_buf_simple_reserve: (0x20006930) buf 0x20009e70 reserve 14
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 20
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 8
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 44
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
[net/buf] [DBG] net_buf_simple_add: (0x20006930) buf 0x20009e70 len 1
...

[jukkar]

This is indeed quite wasteful. The original reason for this was that the code tries to avoid allocating zeroed buffer from stack and then appending that buffer in one go. So this is a trade off between stack usage vs. calling append_u8 multiple times.

[therealprof]

How about a function to add a specified number of zeros? That'd at least run in an inner loop rather than an outer loop with an outlandish number of trivial function calls (+ log output).

[jukkar]

That kind of function would indeed make sense here, would you be able to send a PR for that?

[therealprof]

Sure, where would I add that? IIRC there're a few indirections in there before the call ends up at net_buf_simple_add so we potentially end up with more than one additional function so we can use it from the DHCP client.

[jukkar]

The new function would be either in buf.h or net_pkt.h. Perhaps it makes sense to place it to buf.h so it is more generic, the name could be net_buf_mem_set() or similar. It could look like this:

int net_buf_mem_set(struct net_buf *frag, u16_t offset, u16_t len, u8_t value, s32_t timeout);

Note that the function must work across a chain of net_buf fragments.

[therealprof]

Well, that was the point I was trying to make: I think it belongs into buf, however most subsystems are using the net_pkt class of functions so just having the raw functionality won't help much. We'll either need multiple functions or need to have the net_pkt function do some ugly stuff with the raw buffers.

[jukkar]

Hmm, after looking how the net_pkt_append() is doing it, I must say you are right. Making this with just plain net_buf looks quite challenging from net_pkt point of view. We would probably need support for this functionality both in buf.h and in net_pkt.h