From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 85DE82BB5 for ; Tue, 16 Jan 2018 12:32:51 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jan 2018 03:32:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,368,1511856000"; d="scan'208";a="20192895" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.48]) ([10.237.220.48]) by FMSMGA003.fm.intel.com with ESMTP; 16 Jan 2018 03:32:49 -0800 To: Vipin Varghese , dev@dpdk.org Cc: deepak.k.jain@intel.com, amol.patel@intel.com References: <1512071396-10653-1-git-send-email-vipin.varghese@intel.com> <1513872060-4758-1-git-send-email-vipin.varghese@intel.com> From: Ferruh Yigit Message-ID: Date: Tue, 16 Jan 2018 11:32:48 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1513872060-4758-1-git-send-email-vipin.varghese@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] net/tap: allow user mac to be passed as args 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: Tue, 16 Jan 2018 11:32:51 -0000 On 12/21/2017 4:01 PM, Vipin Varghese wrote: > Added fixes for user TAP user MAC > 1) user format to RTE_PMD_REGISTER_PARAM_STRING > 2) TAP to the RTE_LOG in absence of dynamic RTE_LOG > 3) Boundary case for MAC string added > > --------------------------------------------------------------- > > Other Details: > 1) not to extract "string to mac" conversion to its own function > 2) set_mac_type does not take any pmd or device name > 3) Segault for boundary cases 'mac="01:01:01:01:01:01', not found > 4) Added user MAC format string Hi Vipin, Patch version changes shouldn't be part of git commit. But it is good to put them after "---" in commit log. Can you please update the commit log? Also please add maintainer to the cc/to . > > Signed-off-by: Vipin Varghese <...> > - strncpy(macTemp, value, 18); > - > - macByte = strtok(macTemp, ":"); > - while ((macByte != NULL) && > - (strspn(macByte, "0123456789ABCDEFabcdef")) && > - (strspn((macByte + 1), "0123456789ABCDEFabcdef")) && > - (strlen(macByte) == 2)) { > - user_mac[index] = strtoul(macByte, NULL, 16); > - macByte = strtok(NULL, ":"); > + strncpy(mac_temp, value, 18); > + mac_temp[19] = "\0"; This cause a build error [1], should be used '\0': [1] ...drivers/net/tap/rte_eth_tap.c: In function ‘set_mac_type’: ...drivers/net/tap/rte_eth_tap.c:1467:18: error: assignment makes integer from pointer without a cast [-Werror=int-conversion] mac_temp[19] = "\0"; ^ <....> > - ETH_TAP_IFACE_ARG "= " > - ETH_TAP_SPEED_ARG "= " > - ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED " " > - ETH_TAP_REMOTE_ARG "="); > + ETH_TAP_IFACE_ARG "= " > + ETH_TAP_SPEED_ARG "= " > + ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " " > + ETH_TAP_REMOTE_ARG "="); This also build a build error [2] because of how ETH_TAP_USER_MAC_FMT defined [3]. [2] .../drivers/net/tap/rte_eth_tap.c: At top level: .../drivers/net/tap/rte_eth_tap.c:45:33: error: called object is not a function or function pointer #define ETH_TAP_IFACE_ARG "iface" ^ .../build/include/rte_dev.h:265:25: note: in definition of macro ‘RTE_PMD_REGISTER_PARAM_STRING’ __attribute__((used)) = str ^~~ .../drivers/net/tap/rte_eth_tap.c:1628:2: note: in expansion of macro ‘ETH_TAP_IFACE_ARG’ ETH_TAP_IFACE_ARG "= " ^~~~~~~~~~~~~~~~~ .../drivers/net/tap/rte_eth_tap.c:1630:65: error: expected ‘,’ or ‘;’ before string constant ETH_TAP_MAC_ARG "=" ETH_TAP_MAC_FIXED "|" ETH_TAP_USER_MAC_FMT " " ^ .../build/include/rte_dev.h:265:25: note: in definition of macro ‘RTE_PMD_REGISTER_PARAM_STRING’ __attribute__((used)) = str ^~~ [3] #define ETH_TAP_USER_MAC_FMT ("xx:xx:xx:xx:xx:xx") parenthesis ...