From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ig0-f182.google.com (mail-ig0-f182.google.com [209.85.213.182]) by dpdk.org (Postfix) with ESMTP id 10ECF377A for ; Tue, 7 Apr 2015 20:46:05 +0200 (CEST) Received: by ignm3 with SMTP id m3so15031958ign.0 for ; Tue, 07 Apr 2015 11:46:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=ROo6qpT0ECbStKwe6stsDco1dr4JnkonBHt6S7wsupk=; b=OZL+pful1ilNMzZ8vRJnCXcDB25YqFZN8EvQphSJkw4tjWZayOCDqkOVgvA4zyJse1 yG6Oju5myldS9IZOtTD5pqNUswiVkCgbrnNratJkb0LGYxua/IpRNjUPGhYiYxovKyCJ InnsS+gHdzj29HEHw0iT0pMyqhzxM5cTd7k8Jyz3bIkSvSvcTAGRiyktmr0TdW/TESx2 /bqTd+VYyctKuOF1m3UEHNn4Pr8EIqIPTR/Ex1s9GnKQ5aJ7+uKkmMhrR7Upn7pqVlju Wpd3o78WkTkoyG54Ah0Fgbw8BeXT3jmAQ6MTDkD6xB/HOWAh4s3c0EEgjnC+r0sB3GlV dfNw== X-Gm-Message-State: ALoCoQmy1sBWT2TAzzZXa8eFRYKkwk2UqD76kJTiWoKgl5iSN/2BW4O55JeBXg3NVhifkyTcAp6J MIME-Version: 1.0 X-Received: by 10.107.35.70 with SMTP id j67mr32552315ioj.51.1428432364474; Tue, 07 Apr 2015 11:46:04 -0700 (PDT) Received: by 10.64.58.129 with HTTP; Tue, 7 Apr 2015 11:46:04 -0700 (PDT) In-Reply-To: References: <1428343496-26532-1-git-send-email-stephen@networkplumber.org> Date: Tue, 7 Apr 2015 11:46:04 -0700 Message-ID: From: Stephen Hemminger To: Don Provan Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] eth_dev: make ether dev_ops const X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Apr 2015 18:46:05 -0000 On Tue, Apr 7, 2015 at 10:21 AM, Don Provan 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.