Question about tx/rx buffers

[wegul]

It seems the tx/rx processing is always page-aligned and that each packet will consume strictly one page buffer. In addition, I think the jumbo is handled by enlarging page size. My questions are:

1. Does it make sense to save memory footprint, say merging packets into one page or sticking with 4KB pages? I think Mellanox driver did the latter but im not sure if merging would help gain some performance?
2. In line376, mqnic_rx.c, it says skb_shinfo(skb)->nr_frags = 1; I think it should be nr_frags = 0?
3. In mqnic_map_skb(...), mqnic_tx.c, frags can consume multiple descriptors while in mqnic_start_xmit(...), ring->prod_ptr only incremented by one. Will it cause non-linear frags not being transmitted?

[alexforencich]

This is definitely something that could be improved. Some network device drivers split pages and recycle buffers, but this seems to involve some screwing around with page reference counts and the like, which is TBH outside of my area of expertise. On the RX side, it might be worth experimenting with using multiple 4K pages instead of order 1 or order 2 pages. I'm not sure how much difference it will make though, as with jumbo frames disabled it's going to use 4k pages anyway. Also, there is potentially a concern about XDP, which I think might only work with 4K pages (but I'm also not an expert on XDP).

And I also think split pages is less about saving memory footprint, and more about saving on memory management overhead (calls to allocate and free pages).

It should be 1, because we're attaching one page as a frag. nr_frags = 0 only if there are no extra pages attached to the skb, and doing that would presumably copying the packet data. Although, I wonder if a single page could be converted to an skb, assuming there is enough room for the struct at the end of the page (I think that's how SKBs are usually laid out, with the SKB struct at the end of the buffer). Not sure, but might be an interesting thing to try.

So, this last thing is a bit of a hacky way to implement scatter/gather. There is a field in the queue state that specifies the descriptor block size, 1, 2, 4, or 8 pointers. This "block" is treated as a single descriptor. The plan is to replace this with proper variable-length descriptors at some point, but that hasn't happened yet..... Anyway, since 1 block = 1 descriptor, the pointers are incremented by 1 even if multiple pointers are used.

[wegul]

Thank you Dr.Forencich! This really clarifies my concerns.