From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v1 2/3] net/hyperv: implement core functionality Date: Mon, 18 Dec 2017 21:21:39 +0100 Message-ID: <20171218202139.GE4062@6wind.com> References: <20171124172132.GW4062@6wind.com> <20171218162443.12971-1-adrien.mazarguil@6wind.com> <20171218162443.12971-3-adrien.mazarguil@6wind.com> <20171218102629.43798de2@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Ferruh Yigit , dev@dpdk.org To: Stephen Hemminger Return-path: Received: from mail-wr0-f169.google.com (mail-wr0-f169.google.com [209.85.128.169]) by dpdk.org (Postfix) with ESMTP id D2BBE374E for ; Mon, 18 Dec 2017 21:21:51 +0100 (CET) Received: by mail-wr0-f169.google.com with SMTP id u19so9181820wrc.3 for ; Mon, 18 Dec 2017 12:21:51 -0800 (PST) Content-Disposition: inline In-Reply-To: <20171218102629.43798de2@xeon-e3> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Dec 18, 2017 at 10:26:29AM -0800, Stephen Hemminger wrote: > On Mon, 18 Dec 2017 17:46:23 +0100 > Adrien Mazarguil wrote: > > > +static int > > +ether_addr_from_str(struct ether_addr *eth_addr, const char *str) > > +{ > > + static const uint8_t conv[0x100] = { > > + ['0'] = 0x80, ['1'] = 0x81, ['2'] = 0x82, ['3'] = 0x83, > > + ['4'] = 0x84, ['5'] = 0x85, ['6'] = 0x86, ['7'] = 0x87, > > + ['8'] = 0x88, ['9'] = 0x89, ['a'] = 0x8a, ['b'] = 0x8b, > > + ['c'] = 0x8c, ['d'] = 0x8d, ['e'] = 0x8e, ['f'] = 0x8f, > > + ['A'] = 0x8a, ['B'] = 0x8b, ['C'] = 0x8c, ['D'] = 0x8d, > > + ['E'] = 0x8e, ['F'] = 0x8f, [':'] = 0x40, ['-'] = 0x40, > > + ['\0'] = 0x60, > > + }; > > + uint64_t addr = 0; > > + uint64_t buf = 0; > > + unsigned int i = 0; > > + unsigned int n = 0; > > + uint8_t tmp; > > + > > + do { > > + tmp = conv[(int)*(str++)]; > > + if (!tmp) > > + return -EINVAL; > > + if (tmp & 0x40) { > > + i += (i & 1) + (!i << 1); > > + addr = (addr << (i << 2)) | buf; > > + n += i; > > + buf = 0; > > + i = 0; > > + } else { > > + buf = (buf << 4) | (tmp & 0xf); > > + ++i; > > + } > > + } while (!(tmp & 0x20)); > > + if (n > 12) > > + return -EINVAL; > > + i = RTE_DIM(eth_addr->addr_bytes); > > + while (i) { > > + eth_addr->addr_bytes[--i] = addr & 0xff; > > + addr >>= 8; > > + } > > + return 0; > > +} > > + > > > Why not ether_ntoa? Good question. For the following reasons: - I forgot about the existence of ether_ntoa() and didn't look it up seeing struct ether_addr is (re-)defined by rte_ether.h. What happens when one includes netinet/ether.h together with that file results in various conflicts that trigger a compilation error. This problem should be addressed first. - ether_ntoa() returns a static buffer and is not reentrant, ether_ntoa_r() is but as a GNU extension, I'm not sure it exists on other OSes. Even if this driver is currently targeted at Linux, this is likely not the case for other DPDK code relying on rte_ether.h. - I had ether_addr_from_str()'s code already ready and lying around for a future update in testpmd's flow command parser. No other MAC-48 conversion function I know of is as flexible as this version. The ability to omit ":" and entering partial addresses is a big plus IMO. I think both can coexist on their own merits. Since rte_ether.h needs to be fixed either way, how about I move this function in a separate commit and address the conflict with netinet/ether.h while there? -- Adrien Mazarguil 6WIND