Re: [v2 22/23] Packet Framework IPv4 pipeline sample app

[Olivier MATZ <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>]

Cristian,

Please see some comments below.

On 06/09/2014 03:25 PM, Dumitrescu, Cristian wrote:
> 1. As the name implies, pktmbuf should be used for packets and ctrlmbuf
> should be used for control messages . IMHO using pktmbuf to control
> messages is a confusing workaround.

If ctrlmbuf are removed, the name pktmbuf would change in mbuf.
But anyway, to me it's not confusing at all to store data in a packet,
even if it's a data from a core to another.

> 2. Pktmbuf has a lot of overhead that is not needed in order to send
> short messages between cores. Pktmbuf has a lot of pointers and other
> fields that do not make sense for messages. I don't think we want people
> to say DPDK is difficult to use because e.g. sending 2 bytes from core A
> to core B requires initializing a bunch of pointers and other fields
> that do not make sense.

All the fields that should be initialized in a packet mbuf are
reset in rte_pktmbuf_reset() so the user won't have anything to do.
But using pktmbuf is not the only solution, you can use a private
application structure without duplicating code (see below).

> 3. Once we start using pktmbuf to send messages, it is likely that other
> people will follow this example, and they might do it incorrectly. I
> don't think we want to see emails on this list from people asking e.g:
>
> i) Why does my app segfaults, when all I want to do is send 2 bytes from
> core A to core B?
>
> ii) Why does my app segfaults when core A writes a message to a NIC TX
> queue?

Why would the application segfaults? Indeed, if you misuse any function,
it could segfault but is it a reason for not implementing the feature?

> Using an app dependent structure requires duplicating the work to
> create/free the pool of such structures, and alloc/free mechanism. And
> then some people will ask why are we not using ctrlmbuf, as long as
> ctrlmbuf exists in DPDK.

In this case, I would say that rte_mempool functions are enough to
allocate/free. If the ctrlmbuf structure is composed of a data array
and a length field, you only need:

   rte_mempool_get(mp, &ctrlmbuf);
   memcpy(ctrlmbuf->buf, my_data, my_data_len);
   ctrlmbuf->len = my_data_len;

> I think that, as long as we have ctrlmbuf and pktmbuf in DPDK, we should
> follow the existing model. We should not look for workarounds that we
> know we plan to change anyway, we should look for the right solution. We
> both agree we need to refresh pktmbuf and ctrlmbuf, but my point is we
> should not do changes as long as we don't know what the agreed solution
> will look like?

I agree, we should debate on what is the right solution, that's
precisely what I'm doing. To decide if ctrlmbuf should be kept
or changed, we should:
- understand its use-case if any and see what ctrlmbuf features
   are required
- understand why it should be included in rte_mbuf or not: in my opinion
   there is no reason to do it, and it this has a cost (ex: 1 byte lost
   in mbuf, mbuf fields badly organized)


Regards,
Olivier