From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id C47FC5593 for ; Wed, 25 Apr 2018 14:03:39 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Apr 2018 05:03:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,326,1520924400"; d="scan'208";a="223270635" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.42]) ([10.237.221.42]) by fmsmga006.fm.intel.com with ESMTP; 25 Apr 2018 05:03:37 -0700 To: Qi Zhang , adrien.mazarguil@6wind.com Cc: thomas@monjalon.net, dev@dpdk.org References: <20180425014929.72014-1-qi.z.zhang@intel.com> <20180425074313.242146-1-qi.z.zhang@intel.com> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= xsFNBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABzSVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+wsF+BBMBAgAoAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCWZR3VQUJB33WBQAKCRD5M+tD3xNhH6DWEACVhEb8q1epPwZrUDoxzu7E TS1b8tmabOmnjXZRs6+EXgUVHkp2xxkCfDmL3pa5bC0G/74aJnWjNsdvE05V1cb4YK4kRQ62 FwDQ+hlrFrwFB3PtDZk1tpkzCRHvJgnIil+0MuEh32Y57ig6hy8yO8ql7Lohyrnpfk/nNpm4 jQGEF5qEeHcEFe1AZQlPHN/STno8NZSz2nl0b2cw+cujN1krmvB52Ah/2KugQ6pprVyrGrzB c34ZQO9OsmSjJlETCZk6EZzuhfe16iqBFbOSadi9sPcJRwaUQBid+xdFWl7GQ8qC3zNPibSF HmU43yBZUqJDZlhIcl6/cFpOSjv2sDWdtjEXTDn5y/0FsuY0mFE78ItC4kCTIVk17VZoywcd fmbbnwOSWzDq7hiUYuQGkIudJw5k/A1CMsyLkoUEGN3sLfsw6KASgS4XrrmPO4UVr3mH5bP1 yC7i1OVNpzvOxtahmzm481ID8sk72GC2RktTOHb0cX+qdoiMMfYgo3wRRDYCBt6YoGYUxF1p msjocXyqToKhhnFbXLaZlVfnQ9i2i8jsj9SKig+ewC2p3lkPj6ncye9q95bzhmUeJO6sFhJg Hiz6syOMg8yCcq60j07airybAuHIDNFWk0gaWAmtHZxLObZx2PVn2nv9kLYGohFekw0AOsIW ta++5m48dnCoAc7BTQRX1ky+ARAApzQNvXvE2q1LAS+Z+ni2R13Bb1cDS1ZYq1jgpR13+OKN ipzd8MPngRJilXxBaPTErhgzR0vGcNTYhjGMSyFIHVOoBq1VbP1a0Fi/NqWzJOowo/fDfgVy K4vuitc/gCJs+2se4hdZA4EQJxVlNM51lgYDNpjPGIA43MX15OLAip73+ho6NPBMuc5qse3X pAClNhBKfENRCWN428pi3WVkT+ABRTE0taxjJNP7bb+9TQYNRqGwnGzX5/XISv44asWIQCaq vOkXSUJLd//cdVNTqtL1wreCVVR5pMXj7VIrlk07fmmJVALCmGbFr53BMb8O+8dgK2A5mitM n44d+8KdJWOwziRxcaMk/LclmZS3Iv1TERtiWt98Y9AjeAtcgYPkA3ld0BcUKONogP8pHVz1 Ed3s5rDQ91yr1S0wuAzW91fxGUO4wY+uPmxCtFVuBgd9VT9NAKTUL0qHM7CDgCnZPe0TW6Zj 8OqtdCCyAfvU9cW5xWM7Icxhde6AtPxhDSBwE8fL2ZmrDmaA4jmUKXp3i4JxRPSX84S08b+s DWXHPxy10UFU5A7EK/BEbZAKBwn9ROfm+WK+6X5xOGLoRE++OqNuUudxC1GDyLOPaqCbBCS9 +P6HsTHzxsjyJa27n4jcrcuY3P9TEcFJYSZSeSDh8mVGvugi0exnSJrrBZDyVCcAEQEAAcLB ZQQYAQIADwIbDAUCWZR1ZwUJA59cIQAKCRD5M+tD3xNhH5b+D/9XG44Ci6STdcA5RO/ur05J EE3Ux1DCHZ5V7vNAtX/8Wg4l4GZfweauXwuJ1w7Sp7fklwcNC6wsceI+EmNjGMqfIaukGetG +jBGqsQ7moOZodfXUoCK98gblKgt/BPYMVidzlGC8Q/+lZg1+o29sPnwImW+MXt/Z5az/Z17 Qc265g+p5cqJHzq6bpQdnF7Fu6btKU/kv6wJghENvgMXBuyThqsyFReJWFh2wfaKyuix3Zyj ccq7/blkhzIKmtFWgDcgaSc2UAuJU+x9nuYjihW6WobpKP/nlUDu3BIsbIq09UEke+uE/QK+ FJ8PTJkAsXOf1Bc2C0XbW4Y2hf103+YY6L8weUCBsWC5VH5VtVmeuh26ENURclwfeXhWQ9Og 77yzpTXWr5g1Z0oLpYpWPv745J4bE7pv+dzxOrFdM1xNkzY2pvXph/A8OjxZNQklDkHQ7PIB Lki5L2F4XkEOddUUQchJwzMqTPsggPDmGjgLZrqgO+s4ECZK5+nLD3HEpAbPa3JLDaScy+90 Nu1lAqPUHSnP3vYZVw85ZYm6UCxHE4VLMnnJsN09ZhsOSVR+GyP5Nyw9rT1V3lcsuH7M5Naa 2Xobn9m7l9bRCD/Ji8kG15eV1WTxx1HXVQGjdUYDI7UwegBNbwMLh17XDy+3sn/6SgcqtECA Q6pZKA2mTQxEKMLBZQQYAQIADwIbDAUCWZR3hQUJA59eRwAKCRD5M+tD3xNhH4a/D/4jLAZu UhvU1swWcNEVVCELZ0D3LOV14XcY2MXa3QOpeZ9Bgq7YYJ4S5YXK+SBQS0FkRZdjGNvlGZoG ZdpU+NsQmQFhqHGwX0IT9MeTFM8uvKgxNKGwMVcV9g0IOqwBhGHne+BFboRA9362fgGW5AYQ zT0mzzRKEoOh4r3AQvbM6kLISxo0k1ujdYiI5nj/5WoKDqxTwwfuN1uDUHsWo3tzenRmpMyU NyW3Dc+1ajvXLyo09sRRq7BnM99Rix1EGL8Qhwy+j0YAv+FuspWxUX9FxXYho5PvGLHLsHfK FYQ7x/RRbpMjkJWVfIe/xVnfvn4kz+MTA5yhvsuNi678fLwY9hBP0y4lO8Ob2IhEPdfnTuIs tFVxXuelJ9xAe5TyqP0f+fQjf1ixsBZkqOohsBXDfje0iaUpYa/OQ/BBeej0dUdg2JEu4jAC x41HpVCnP9ipLpD0fYz1d/dX0F/VY2ovW6Eba/y/ngOSAR6C+u881m7oH2l0G47MTwkaQCBA bLGXPj4TCdX3lftqt4bcBPBJ+rFAnJmRHtUuyyaewBnZ81ZU2YAptqFM1kTh+aSvMvGhfVsQ qZL2rk2OPN1hg+KXhErlbTZ6oPtLCFhSHQmuxQ4oc4U147wBTUuOdwNjtnNatUhRCp8POc+3 XphVR5G70mnca1E2vzC77z+XSlTyRA== Message-ID: <294cd4dd-727d-c219-2b7d-a09ca0dfb1c2@intel.com> Date: Wed, 25 Apr 2018 13:03:36 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180425074313.242146-1-qi.z.zhang@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd failure due to RSS offload check 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: Wed, 25 Apr 2018 12:03:40 -0000 On 4/25/2018 8:43 AM, Qi Zhang wrote: > After add RSS hash offload check, default rss_hf will fail on > devices that not support all bits, the patch introduce RSS > negotiate flag, when it is set, RSS configuration will negotiate > with device capability. By default negotiate is turn on, so it > will not break exist PMD. It can be disabled by "--disable-rss-neg" > in start parameters or be enable/disable by testpmd command > "port config all rss neg|noneg". Hi Qi, Thanks for the patch, but I feel new neg|noneg is extra complexity, I am wondering if we can fix this without this complexity. "rss_hf" is rss hash function configuration for *all* ports. Default value is ETH_RSS_IP. Up until recently it was more like default value, Adrien's commit start updating it in "port config all rss ..." and it become more like config value now. And if it is config value, the question is what to do if PMD doesn't support requested hash function: - Fail - Convert request what PMD supports. init_port_config() called from many places and it sets rss_conf.rss_hf via "rss_hf" and currently this is the problem some pmds hit. Previously, before Adrien's update, when "port config all rss ..." failed it was printing log and keep continue, not recording the failed value as config value. What do you think: - In init_port_config() use "rss_hf & dev_info.flow_type_rss_offloads" as you suggested. - in cmd_config_rss_parsed() set "rss_hf" only if all ethdev successfully called rte_eth_dev_rss_hash_update(), this will mean the config has supported by all ports so it will be ok to use is as "rss_hf & dev_info.flow_type_rss_offloads" in init_port_config() - I think there is a defect in "port config all rss default", perhaps because of me during merged, it shouldn't update "rss_hf" After this changes answer to above question, by default config behavior is "Convert request what PMD supports" so testpmd don't fail, and when configuration updated only values supported by ports accepted. > > Fixes: 527624c663f8 ("ethdev: add supported hash function check") > Signed-off-by: Qi Zhang > --- > > v2: > - fix command help string. > > app/test-pmd/cmdline.c | 22 ++++++++++++++++------ > app/test-pmd/parameters.c | 5 +++++ > app/test-pmd/testpmd.c | 12 +++++++++++- > app/test-pmd/testpmd.h | 1 + > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +++- > 5 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index 333852194..9390be741 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -822,7 +822,7 @@ static void cmd_help_long_parsed(void *parsed_result, > " for ports.\n\n" > > "port config all rss (all|default|ip|tcp|udp|sctp|" > - "ether|port|vxlan|geneve|nvgre|none|)\n" > + "ether|port|vxlan|geneve|nvgre|none|neg|noneg|)\n" > " Set the RSS mode.\n\n" > > "port config port-id rss reta (hash,queue)[,(hash,queue)]\n" > @@ -2029,7 +2029,13 @@ cmd_config_rss_parsed(void *parsed_result, > else if (isdigit(res->value[0]) && atoi(res->value) > 0 && > atoi(res->value) < 64) > rss_conf.rss_hf = 1ULL << atoi(res->value); > - else { > + else if (!strcmp(res->value, "neg")) { > + rss_hf_noneg = 0; > + return; > + } else if (!strcmp(res->value, "noneg")) { > + rss_hf_noneg = 1; > + return; > + } else { > printf("Unknown parameter\n"); > return; > } > @@ -2037,10 +2043,14 @@ cmd_config_rss_parsed(void *parsed_result, > /* Update global configuration for RSS types. */ > rss_hf = rss_conf.rss_hf; > RTE_ETH_FOREACH_DEV(i) { > - if (!strcmp(res->value, "default")) { > - rte_eth_dev_info_get(i, &dev_info); > + rte_eth_dev_info_get(i, &dev_info); > + if (!strcmp(res->value, "default")) > rss_conf.rss_hf = dev_info.flow_type_rss_offloads; > - } > + else > + rss_conf.rss_hf &= (rss_hf_noneg ? > + rss_conf.rss_hf : > + dev_info.flow_type_rss_offloads); > + > diag = rte_eth_dev_rss_hash_update(i, &rss_conf); > if (diag < 0) > printf("Configuration of RSS hash at ethernet port %d " > @@ -2064,7 +2074,7 @@ cmdline_parse_inst_t cmd_config_rss = { > .f = cmd_config_rss_parsed, > .data = NULL, > .help_str = "port config all rss " > - "all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|", > + "all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|noneg|", > .tokens = { > (void *)&cmd_config_rss_port, > (void *)&cmd_config_rss_keyword, > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c > index 394fa6d92..4758ec1d9 100644 > --- a/app/test-pmd/parameters.c > +++ b/app/test-pmd/parameters.c > @@ -139,6 +139,7 @@ usage(char* progname) > printf(" --enable-hw-vlan-extend: enable hardware vlan extend.\n"); > printf(" --enable-drop-en: enable per queue packet drop.\n"); > printf(" --disable-rss: disable rss.\n"); > + printf(" --disable-rss-neg: disable rss negotiate with device capa.\n"); > printf(" --port-topology=N: set port topology (N: paired (default) or " > "chained).\n"); > printf(" --forward-mode=N: set forwarding mode (N: %s).\n", > @@ -594,6 +595,7 @@ launch_args_parse(int argc, char** argv) > { "enable-hw-vlan-extend", 0, 0, 0 }, > { "enable-drop-en", 0, 0, 0 }, > { "disable-rss", 0, 0, 0 }, > + { "disable-rss-neg", 0, 0, 0 }, > { "port-topology", 1, 0, 0 }, > { "forward-mode", 1, 0, 0 }, > { "rss-ip", 0, 0, 0 }, > @@ -909,6 +911,9 @@ launch_args_parse(int argc, char** argv) > > if (!strcmp(lgopts[opt_idx].name, "disable-rss")) > rss_hf = 0; > + if (!strcmp(lgopts[opt_idx].name, "disable-rss-neg")) { > + rss_hf_noneg = 1; > + } > if (!strcmp(lgopts[opt_idx].name, "port-topology")) { > if (!strcmp(optarg, "paired")) > port_topology = PORT_TOPOLOGY_PAIRED; > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index d6da41927..68214677c 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -257,6 +257,12 @@ int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET; > uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */ > > /* > + * Negotiate with device capability when config > + * Receive Side Scaling (RSS). > + */ > +uint8_t rss_hf_noneg = 0; /* Negotiate by default */ > + > +/* > * Port topology configuration > */ > uint16_t port_topology = PORT_TOPOLOGY_PAIRED; /* Ports are paired by default */ > @@ -2265,13 +2271,17 @@ init_port_config(void) > { > portid_t pid; > struct rte_port *port; > + struct rte_eth_dev_info dev_info; > > RTE_ETH_FOREACH_DEV(pid) { > port = &ports[pid]; > port->dev_conf.fdir_conf = fdir_conf; > if (nb_rxq > 1) { > + rte_eth_dev_info_get(pid, &dev_info); > port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL; > - port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf; > + port->dev_conf.rx_adv_conf.rss_conf.rss_hf = > + rss_hf_noneg ? rss_hf : > + (rss_hf & dev_info.flow_type_rss_offloads); > } else { > port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL; > port->dev_conf.rx_adv_conf.rss_conf.rss_hf = 0; > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h > index 070919822..c37c4ee70 100644 > --- a/app/test-pmd/testpmd.h > +++ b/app/test-pmd/testpmd.h > @@ -385,6 +385,7 @@ extern struct rte_eth_rxmode rx_mode; > extern struct rte_eth_txmode tx_mode; > > extern uint64_t rss_hf; > +extern uint8_t rss_hf_noneg; > > extern queueid_t nb_rxq; > extern queueid_t nb_txq; > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > index 593b13a3d..8db9cf5c1 100644 > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > @@ -1751,13 +1751,15 @@ port config - RSS > > Set the RSS (Receive Side Scaling) mode on or off:: > > - testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none) > + testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|noneg) > > RSS is on by default. > > The ``all`` option is equivalent to ip|tcp|udp|sctp|ether. > The ``default`` option enables all supported RSS types reported by device info. > The ``none`` option is equivalent to the ``--disable-rss`` command-line option. > +The ``neg`` option enable the negotiation with device capability when configure RSS. > +The ``noneg`` option disable the negotiation and is equivalent to ``--disable-rss-neg`` command-line option. > > port config - RSS Reta > ~~~~~~~~~~~~~~~~~~~~~~ >