Re: [dpdk-dev] [PATCH] eth_dev: make ether dev_ops const

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, Apr 7, 2015 at 10:21 AM, Don Provan <dprovan@bivio.net> wrote:

> -----Original Message-----
> >From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >Sent: Monday, April 06, 2015 11:05 AM
> >To: dev@dpdk.org
> >Subject: [dpdk-dev] [PATCH] eth_dev: make ether dev_ops const
> >
> >Ethernet device function tables should be immutable for correctness and
> security. Special case for the test code driver.
> ...
> >diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c index
> f163562..f579558 100644
> >--- a/app/test/virtual_pmd.c
> >+++ b/app/test/virtual_pmd.c
> ...
> >+/* This driver uses private mutable eth_dev_ops for each
> >+ * instance so it is safe to override const here.
> >+ */
> >+#pragma GCC diagnostic push
> >+#pragma GCC diagnostic ignored "-Wcast-qual"
> > void
> > virtual_ethdev_start_fn_set_success(uint8_t port_id, uint8_t success)  {
> >       struct rte_eth_dev *vrtl_eth_dev = &rte_eth_devices[port_id];
> >+      struct eth_dev_ops *dev_ops
> >+              = (struct eth_dev_ops *) vrtl_eth_dev->dev_ops;
>  ...
>
> If this is really safe, then you should be able to accomplish it
> without disabling a bunch of protection. I suggest adding a
> pointer that isn't const to the private data block and adjusting
> the allocated dispatch table through that instead of through
> the pointer to the immutable dispatch table you've established
> in struct rte_eth_dev. That reinforces the fact that modifying
> the dispatch table is a private matter within the driver while
> showing structurally exactly why it's safe to change it.
>
> And it's not nearly so ugly.
>
> -don provan
> dprovan@bivio.net
>
>
In this case it is safe, but only because this dummy driver used in testing
does non-standard things.
It copies a base template for ops into a allocated area of memory, then
modifies it.
Not the best design, but did not want to hold back the ethernet dev_ops.
Probably the private pointer is a better way.