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

[Don Provan <dprovan@bivio.net>]

-----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