From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 48356952 for ; Mon, 6 Mar 2017 15:51:48 +0100 (CET) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga104.jf.intel.com with ESMTP; 06 Mar 2017 06:51:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,254,1484035200"; d="scan'208";a="64756472" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga004.jf.intel.com with ESMTP; 06 Mar 2017 06:51:47 -0800 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 6 Mar 2017 06:51:47 -0800 Received: from fmsmsx113.amr.corp.intel.com ([169.254.13.172]) by fmsmsx117.amr.corp.intel.com ([169.254.3.201]) with mapi id 14.03.0248.002; Mon, 6 Mar 2017 06:51:47 -0800 From: "Wiles, Keith" To: Pascal Mazon CC: "dev@dpdk.org" Thread-Topic: [PATCH 1/4] net/tap: move private elements to external header Thread-Index: AQHSlAtoCxer2x0aBk6VRxs3/OLix6GDxlmAgASg0oCAAAk0AA== Date: Mon, 6 Mar 2017 14:51:46 +0000 Message-ID: References: <20743012cc151e3dfd30eff7ef0f6a03b75ac0c9.1488537600.git.pascal.mazon@6wind.com> <20170306151849.7bc1f8ca@paques.dev.6wind.com> In-Reply-To: <20170306151849.7bc1f8ca@paques.dev.6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.49.209] Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 1/4] net/tap: move private elements to external header X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Mar 2017 14:51:49 -0000 > On Mar 6, 2017, at 8:18 AM, Pascal Mazon wrote: >=20 > On Fri, 3 Mar 2017 15:38:11 +0000 > "Wiles, Keith" wrote: >=20 >>=20 >>> On Mar 3, 2017, at 4:45 AM, Pascal Mazon >>> wrote: >>>=20 >>> In the next patch, access to struct pmd_internals will be necessary >>> in tap_flow.c to store the flows. >>>=20 >>> Signed-off-by: Pascal Mazon >>> Acked-by: Olga Shern >>> --- >>> drivers/net/tap/Makefile | 1 + >>> drivers/net/tap/rte_eth_tap.c | 34 ++------------------ >>> drivers/net/tap/tap.h | 73 >>> +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 >>> insertions(+), 32 deletions(-) create mode 100644 >>> drivers/net/tap/tap.h >>>=20 >>> diff --git a/drivers/net/tap/Makefile b/drivers/net/tap/Makefile >>> index e18f30c56f52..bdbe69e62a4e 100644 >>> --- a/drivers/net/tap/Makefile >>> +++ b/drivers/net/tap/Makefile >>> @@ -40,6 +40,7 @@ EXPORT_MAP :=3D rte_pmd_tap_version.map >>> LIBABIVER :=3D 1 >>>=20 >>> CFLAGS +=3D -O3 >>> +CFLAGS +=3D -I$(SRCDIR) >>> CFLAGS +=3D $(WERROR_FLAGS) >>>=20 >>> # >>> diff --git a/drivers/net/tap/rte_eth_tap.c >>> b/drivers/net/tap/rte_eth_tap.c index 3fd057225ab3..fa57d645f3b1 >>> 100644 --- a/drivers/net/tap/rte_eth_tap.c >>> +++ b/drivers/net/tap/rte_eth_tap.c >>> @@ -51,6 +51,8 @@ >>> #include >>> #include >>>=20 >>> +#include >>> + >>> /* Linux based path to the TUN device */ >>> #define TUN_TAP_DEV_PATH "/dev/net/tun" >>> #define DEFAULT_TAP_NAME "dtap" >>> @@ -83,38 +85,6 @@ static struct rte_eth_link pmd_link =3D { >>> .link_autoneg =3D ETH_LINK_SPEED_AUTONEG >>> }; >>>=20 >>> -struct pkt_stats { >>> - uint64_t opackets; /* Number of output >>> packets */ >>> - uint64_t ipackets; /* Number of input >>> packets */ >>> - uint64_t obytes; /* Number of bytes on >>> output */ >>> - uint64_t ibytes; /* Number of bytes on >>> input */ >>> - uint64_t errs; /* Number of error >>> packets */ -}; >>> - >>> -struct rx_queue { >>> - struct rte_mempool *mp; /* Mempool for RX >>> packets */ >>> - uint16_t in_port; /* Port ID */ >>> - int fd; >>> - >>> - struct pkt_stats stats; /* Stats for this >>> RX queue */ -}; >>> - >>> -struct tx_queue { >>> - int fd; >>> - struct pkt_stats stats; /* Stats for this >>> TX queue */ -}; >>> - >>> -struct pmd_internals { >>> - char name[RTE_ETH_NAME_MAX_LEN]; /* Internal Tap >>> device name */ >>> - uint16_t nb_queues; /* Number of queues >>> supported */ >>> - struct ether_addr eth_addr; /* Mac address of the >>> device port */ - >>> - int if_index; /* IF_INDEX for the >>> port */ - >>> - struct rx_queue rxq[RTE_PMD_TAP_MAX_QUEUES]; /* >>> List of RX queues */ >>> - struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* >>> List of TX queues */ -}; >>> - >>> /* Tun/Tap allocation routine >>> * >>> * name is the number of the interface to use, unless NULL to take >>> the host diff --git a/drivers/net/tap/tap.h b/drivers/net/tap/tap.h >>> new file mode 100644 >>> index 000000000000..88f62b895feb >>> --- /dev/null >>> +++ b/drivers/net/tap/tap.h >>> @@ -0,0 +1,73 @@ >>> +/*- >>> + * BSD LICENSE >>> + * >>> + * Copyright 2017 6WIND S.A. >>> + * Copyright 2017 Mellanox. >>> + * >>> + * Redistribution and use in source and binary forms, with or >>> without >>> + * modification, are permitted provided that the following >>> conditions >>> + * are met: >>> + * >>> + * * Redistributions of source code must retain the above >>> copyright >>> + * notice, this list of conditions and the following >>> disclaimer. >>> + * * Redistributions in binary form must reproduce the above >>> copyright >>> + * notice, this list of conditions and the following >>> disclaimer in >>> + * the documentation and/or other materials provided with the >>> + * distribution. >>> + * * Neither the name of 6WIND S.A. nor the names of its >>> + * contributors may be used to endorse or promote products >>> derived >>> + * from this software without specific prior written >>> permission. >>> + * >>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND >>> CONTRIBUTORS >>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT >>> NOT >>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND >>> FITNESS FOR >>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE >>> COPYRIGHT >>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, >>> INCIDENTAL, >>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT >>> NOT >>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS >>> OF USE, >>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED >>> AND ON ANY >>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, >>> OR TORT >>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF >>> THE USE >>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH >>> DAMAGE. >>> + */ >>> + >>> +#ifndef _TAP_H_ >>> +#define _TAP_H_ >>> + >>> +#include >>> + >>> +#include >>> +#include >>> + >>> +#define RTE_PMD_TAP_MAX_QUEUES 16 >>> + >>> +struct pkt_stats { >>> + uint64_t opackets; /* Number of output packets */ >>> + uint64_t ipackets; /* Number of input packets */ >>> + uint64_t obytes; /* Number of bytes on output */ >>> + uint64_t ibytes; /* Number of bytes on input */ >>> + uint64_t errs; /* Number of error packets */ >>> +}; >>> + >>> +struct rx_queue { >>> + struct rte_mempool *mp; /* Mempool for RX packets */ >>> + uint16_t in_port; /* Port ID */ >>> + int fd; >>> + struct pkt_stats stats; /* Stats for this RX queue */ >>> +}; >>> + >>> +struct tx_queue { >>> + int fd; >>> + struct pkt_stats stats; /* Stats for this TX queue */ >>> +}; >>> + >>> +struct pmd_internals { >>> + char name[RTE_ETH_NAME_MAX_LEN]; /* Internal Tap device >>> name */ >>> + uint16_t nb_queues; /* Number of queues supported */ >>> + struct ether_addr eth_addr; /* Mac address of the device >>> port */ >>> + int if_index; /* IF_INDEX for the port */ >>> + struct rx_queue rxq[RTE_PMD_TAP_MAX_QUEUES]; /* List of RX >>> queues */ >>> + struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX >>> queues */ +}; >>=20 >> I guess I am going to be a bit picky here on the formatting. Moving >> the code from .c to .h you compress a lot of white space out and now >> I think it is very hard to read. Can you add back some of the white >> space for readability. >>=20 >=20 > Do you mean whitespaces between ";" and the comment, or do you mean line > jumps? Would you rather have it look like what's described there? > http://dpdk.org/doc/guides/contributing/coding_style.html#structure-decl= arations The spaces between the ; and comment is what I wanted here. The extra blank= lines are optional to break up the structure some is good as well. You do = not need to align the variables in the same columns as long as the comments= are spaced out and aligned would be nice. To me white space is your friend when writing code and the compiler does no= t care normally :-) >=20 > Regards, > Pascal >=20 >>> + >>> +#endif /* _TAP_H_ */ >>> --=20 >>> 2.8.0.rc0 >>>=20 >>=20 >> Regards, >> Keith >>=20 >=20 Regards, Keith