From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 269DC1B016 for ; Tue, 19 Dec 2017 11:02:08 +0100 (CET) Received: by mail-wm0-f42.google.com with SMTP id 64so2351008wme.3 for ; Tue, 19 Dec 2017 02:02:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=kszmHQrX8xEWrbaLQ0/z7E+qtd+Kf5mZp6NfRAUD1YU=; b=0arlxR+fH4DdQXOTkZFhiUNke6MbPxmc8PiN6TEczPMG9Bv9AMWmkx9iy3GQdF4dxc LtyKuU+pMhVPLqX0njoiuEvCjdzRObZM60/0h9jOUZtKJNtE1ujd+XYxHKb6k/rsyq/h CNPXzDNw0x5Om4K6j8MAmZv8Zq3e5w5XeBDqJRAEZ8TQlvfwrG82B6rz1Q7UULmlZduw /akCZLnXoS6I7uuynpTa7ZJyd2lxJsSzdHDyFCp5yben4Xoruj7esByFbrK8wu8QhYQl iyiuaHGyWrFILgJ0OE2z/8PwHmsetwgrqwnca58TES5mtd6h9dvJPTd0/yJxIDuoB6Zp gqIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=kszmHQrX8xEWrbaLQ0/z7E+qtd+Kf5mZp6NfRAUD1YU=; b=e+h+wWDTchgNfB2mrvCODDGpUivtAaJ9fcKumL5pSK4mGY7neaIkzaALKv1z6z8czQ 3YIllsAXkhD753HBHPpQxoNQI0kY4RF8ffeBZ9GAcST2GjBYXoKUc88arx+yNEzJ9QTy Inthoj5sRSZL9kadc7L8B9Fm6GIBEf6Nw1Hcs8GNArM/VXFX8GBSSP6m/P+SN7kYarNk reLdgkAfW52iboiQWm7s/+hWkyGSXs1EBYo0CXnx55dCUp7qzNPq34dqZ2bz4uutjLrz kt6YglPoizECQyhlrafCe8Wxy32CYkTL9rPsfougolaDjhco0L15P2S0ZnygO8iBgo3a wBzQ== X-Gm-Message-State: AKGB3mKRJyBNi6t9jjb3ZRj1Rj/TTytF+BSEBndpda9DRky7O2eLod6e QPod9uT68FL3f0jnN4yST7MArA== X-Google-Smtp-Source: ACJfBovn3tDxahayx/diCGsE8NEU76yJqPEaYwJXMQfOHvYTfqiSqEp63b31rDM/mC55Ge3YL9bB9g== X-Received: by 10.28.144.10 with SMTP id s10mr2827289wmd.103.1513677727862; Tue, 19 Dec 2017 02:02:07 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id g25sm11596788edc.68.2017.12.19.02.02.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 Dec 2017 02:02:07 -0800 (PST) Date: Tue, 19 Dec 2017 11:01:55 +0100 From: Adrien Mazarguil To: Stephen Hemminger Cc: Ferruh Yigit , dev@dpdk.org Message-ID: <20171219100155.GB5377@6wind.com> References: <20171124172132.GW4062@6wind.com> <20171218162443.12971-1-adrien.mazarguil@6wind.com> <20171218162443.12971-3-adrien.mazarguil@6wind.com> <20171218155946.5806589a@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171218155946.5806589a@xeon-e3> Subject: Re: [dpdk-dev] [PATCH v1 2/3] net/hyperv: implement core functionality 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, 19 Dec 2017 10:02:08 -0000 On Mon, Dec 18, 2017 at 03:59:46PM -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++)]; > > Cast to int will cause out of bounds reference on non-ascii strings. > The parser will get confused by: > 001:aa:bb:cc:dd:ee:ff or invalid strings. Nice catch! I added the (int) cast to shut up a GCC complaint about using char as index type. The proper fix taking care of integer conversion and array bounds safety check should read: tmp = conv[*str++ & 0xffu]; > Why not use sscanf which would be safer in this case. Right, this is indeed the obvious implementation, however not only the fixed MAC-48 format is not the most convenient to use for user input (somewhat like forcing them to enter fully expanded IPv6 addresses every time), sscanf() also ignores leading white spaces and successfully parses weird expressions like " -42: 0x66: 0af: 0: 44:-6", which I think is a problem. > /** > * Parse 48bits Ethernet address in pattern xx:xx:xx:xx:xx:xx. > * > * @param eth_addr > * A pointer to a ether_addr structure. > * @param str > * A pointer to string contains the formatted MAC address. > * @return > * 0 if the address is valid > * -EINVAL if address is not formatted properly > */ > static inline int > ether_parse_addr(struct ether_addr *eth_addr, const char *str) > { > int n; > > n = sscanf(str, > "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", > ð_addr->addr_bytes[0], > ð_addr->addr_bytes[1], > ð_addr->addr_bytes[2], > ð_addr->addr_bytes[3], > ð_addr->addr_bytes[4], > ð_addr->addr_bytes[5]); > return (n == ETHER_ADDR_LEN) ? 0 : -EINVAL; > } > > > + 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; > > +} > > + -- Adrien Mazarguil 6WIND