From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1D0FEA04DD; Fri, 23 Oct 2020 18:21:23 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DC6275592; Fri, 23 Oct 2020 18:21:21 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 13A714C97 for ; Fri, 23 Oct 2020 18:21:18 +0200 (CEST) IronPort-SDR: yfc7POQ+RlmDbzmKYgUWOF9YEkGCIDoyTA1UL8vgj2SesIrIbvhetQgtEXGXmOvnp2k2v4kP28 4XPbaaezugSw== X-IronPort-AV: E=McAfee;i="6000,8403,9782"; a="167820059" X-IronPort-AV: E=Sophos;i="5.77,409,1596524400"; d="scan'208";a="167820059" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Oct 2020 09:21:16 -0700 IronPort-SDR: MLHd5FZJ2oeAL7PpLuOkf/x1sVnAugN93kKqYtJKDknbraHmFatQRy0Lv5mEZeC3Yk39LQRkAc ZWlcpelu8ECg== X-IronPort-AV: E=Sophos;i="5.77,409,1596524400"; d="scan'208";a="534452988" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.224.253]) ([10.213.224.253]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Oct 2020 09:21:15 -0700 To: Long Li , Stephen Hemminger Cc: dev@dpdk.org, Stephen Hemminger , Long Li References: <1603395970-26797-1-git-send-email-longli@linuxonhyperv.com> From: Ferruh Yigit Message-ID: <53ea0071-6a22-6efd-f209-f599e1dc1a4d@intel.com> Date: Fri, 23 Oct 2020 17:21:11 +0100 MIME-Version: 1.0 In-Reply-To: <1603395970-26797-1-git-send-email-longli@linuxonhyperv.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 1/2] net/netvsc: allow setting rx and tx copy break 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/22/2020 8:46 PM, Long Li wrote: > From: Stephen Hemminger > > The values for Rx and Tx copy break should be tunable rather > than hard coded constants. > > The rx_copybreak sets the threshold where the driver uses an > external mbuf to avoid having to copy data. Setting 0 for copybreak > will cause driver to always create an external mbuf. Setting > a value greater than the MTU would prevent it from ever making > an external mbuf and always copy. The default value is 256 (bytes). > > Likewise the tx_copybreak sets the threshold where the driver > aggregates multiple small packets into one request. If tx_copybreak > is 0 then each packet goes as a VMBus request (no copying). > If tx_copybreak is set larger than the MTU, then all packets smaller > than the chunk size of the VMBus send buffer will be copied; larger > packets always have to go as a single direct request. The default > value is 512 (bytes). > > Signed-off-by: Stephen Hemminger > Signed-off-by: Long Li <...> > @@ -45,6 +45,10 @@ > DEV_RX_OFFLOAD_VLAN_STRIP | \ > DEV_RX_OFFLOAD_RSS_HASH) > > +#define NETVSC_ARG_LATENCY "latency" > +#define NETVSC_ARG_RXBREAK "rx_copybreak" > +#define NETVSC_ARG_TXBREAK "tx_copybreak" > + Can you please document new devargs in the driver documentation? <...> > @@ -181,12 +167,32 @@ static int hn_parse_args(const struct rte_eth_dev *dev) > return -EINVAL; > } > > - ret = rte_kvargs_process(kvlist, "latency", hn_set_latency, hv); > - if (ret) > - PMD_DRV_LOG(ERR, "Unable to process latency arg\n"); > + for (i = 0; i != kvlist->count; ++i) { > + const struct rte_kvargs_pair *pair = &kvlist->pairs[i]; > + > + if (!strcmp(pair->key, NETVSC_ARG_LATENCY)) > + latency = atoi(pair->value); > + else if (!strcmp(pair->key, NETVSC_ARG_RXBREAK)) > + rx_break = atoi(pair->value); > + else if (!strcmp(pair->key, NETVSC_ARG_TXBREAK)) > + tx_break = atoi(pair->value); > + } > + Instead of accessing to the kvlist internals, I think better to use 'rte_kvargs_process()' as done previously. If the reason to remove callback is to not create a callback for each argument, a generic one can be used for all. > + if (latency >= 0) { > + PMD_DRV_LOG(DEBUG, "set latency %d usec", latency); > + hv->latency = latency * 1000; /* usec to nsec */ > + } > + if (rx_break >= 0) { > + PMD_DRV_LOG(DEBUG, "rx copy break set to %d", rx_break); > + hv->rx_copybreak = rx_break; > + } > + if (tx_break >= 0) { > + PMD_DRV_LOG(DEBUG, "tx copy break set to %d", tx_break); > + hv->tx_copybreak = tx_break; > + } > When 'rte_kvargs_process()' used, the valued can be assigned directly to 'hv->tx_copybreak', if the argument is not available, it won't be updated, so above check can be dropped.