On Tue, Apr 7, 2015 at 10:21 AM, Don Provan <dprovan at bivio.net> wrote:
> -----Original Message----- > >From: Stephen Hemminger [mailto:stephen at networkplumber.org] > >Sent: Monday, April 06, 2015 11:05 AM > >To: dev at 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 at 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.
