From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f179.google.com (mail-qk0-f179.google.com [209.85.220.179]) by dpdk.org (Postfix) with ESMTP id 8D9241B53 for ; Thu, 23 Mar 2017 20:46:34 +0100 (CET) Received: by mail-qk0-f179.google.com with SMTP id p22so50112537qka.3 for ; Thu, 23 Mar 2017 12:46:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=atomicrules-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=QtWaU9/YFCyZz/XHqTwkIyutWPR9ZFTyYcCx4zUjYUI=; b=IXbrRKOlOH5Hpi9M1i+7g2uygyAXSQEiQqHr8Cls2OnWeLJI8eWEaVZm/KcFACDrNg GPk9q/cx5ciekHwEuKLoFfT+/ukBfc1UuroBZLdNdarqdlLw7uthq9xfgFthxRnMShsz WfxNDoy2e1/PMuvq78HC0w+aZVluGaimhuUKhIg792DGNBMagjEnxsaMslxGScod9g/l WJqJeZrclz49aHAzX1UGvbMG6lOqwvTop+htmk06u28C3K+5X9BW0er2LOjHmGkuko41 8V9quHwH4hK6M82fgmM3I1gJTaeu9vcBPHaZxNxqHzIpprdGWHJID8Ui2eSJqk70gRnQ cFtQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=QtWaU9/YFCyZz/XHqTwkIyutWPR9ZFTyYcCx4zUjYUI=; b=sImfnaHANrZDAU75qqj5Q94MS3HZKEJLppsHBitMXDkBas4uBWDb915pEKXk5JZXl2 lWQvPmYI91TXzIzrOKPMUGCpIXAb5sQNuAnLzKhz9xMvXoTPPNTnx4I78o75aG4EBMvf rdQYPQNAVvpcqKP1mrrI6NIwISP6eXUn/xi9n9JrN7DOm+oW204RO7plwR6M9ZNnOghp ed7FxB1P7y/26zsvhq9eNKRZZL5NAtzadufjkwmk3gxI9tvtPx2HzcUGZrZjSpI+iwDc P7XF/kX3vo3zrrKLLzxIa4DzinGKkjBeOxNFi6bhXn43hNUBcwBb7NTjr3dePjMbd4SU MRWA== X-Gm-Message-State: AFeK/H0ATJ54Ym1IDwNFIp0pJGS575UeDu1OeWyPayxf1o12bKhyYqFv9uXidDEtk361MJK//OXl/p0PM1AXrg== X-Received: by 10.55.151.3 with SMTP id z3mr4003237qkd.79.1490298393701; Thu, 23 Mar 2017 12:46:33 -0700 (PDT) MIME-Version: 1.0 Received: by 10.200.1.131 with HTTP; Thu, 23 Mar 2017 12:46:13 -0700 (PDT) In-Reply-To: <0d1c21b6-6163-12f0-cbe4-d23c06a809e9@intel.com> References: <1490231015-31748-1-git-send-email-ed.czeck@atomicrules.com> <0d1c21b6-6163-12f0-cbe4-d23c06a809e9@intel.com> From: Ed Czeck Date: Thu, 23 Mar 2017 15:46:13 -0400 Message-ID: To: Ferruh Yigit Cc: dev@dpdk.org Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v4 1/7] net/ark: PMD for Atomic Rules Arkville driver stub 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: Thu, 23 Mar 2017 19:46:34 -0000 In the next patch set, v5 the following are addressed. > > > > +CONFIG_RTE_LIBRTE_ARK_PMD=n > Is it not tested or known that it is not supported? > > CONFIG_RTE_LIBRTE_IXGBE_PMD=n > > CONFIG_RTE_LIBRTE_I40E_PMD=n > > CONFIG_RTE_LIBRTE_VIRTIO_PMD=y The Ark/PMD is not supported on those architectures at this time. This is also reflected in doc/guides/nic/ark.rst > Can you also please document the device arguments in this file? > Device Parameters > ----------------- > "Pkt_gen" > "Pkt_chkr" > "Pkt_dir" doc/guides/nic/ark.rst has been update to include documentation for these arguments. > > diff --git a/doc/guides/nics/features/ark.ini b/doc/guides/nics/features/ark.ini > ... > Features can be added with the patch that adds functionality. I believe > above features not supported with current patch. The ark.ini file has been removed in this patch and will be added in a later patch > > +# > > +SRCS-y += ark_ethdev.c > Please use SRCS-y only in comment, for actual usage please prefer: > SRCS-$(CONFIG_RTE_LIBRTE_ARK_PMD) Changed per request. > > > +#define ARK_TRACE_ON(fmt, ...) \ > > + PMD_DRV_LOG(ERR, fmt, ##__VA_ARGS__) > > + > > +#define ARK_TRACE_OFF(fmt, ...) \ > > + do {if (0) PMD_DRV_LOG(ERR, fmt, ##__VA_ARGS__); } while (0) > why not just "do { } while(0)" ? A do while body always executes at least once. The if (0) is required. > This is debug option but prints only "ERR" level log, shouldn't this be > DEBUG. > Also there are dpdk wide log level option, helps optimizing out some > code, if you use only ERR type, you won't be benefiting from it. Changed to DEBUG > > __rte_unused not required. Removed. > > + ARK_DEBUG_TRACE("eth_ark_dev_init(struct rte_eth_dev *dev)\n"); > > + gark[0] = ark; > Is it OK to have this hardcoded index "0"? When there are two instance > of this device, this value be overwritten by second one. Code refactored moving this variable from global scope to instance specific. > dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; I do not understand your comment. > > memset not required, since already done by ethdev abstraction layer, > specially desc_lim values already overwritten below. Deleted. > > > + dev_info->max_rx_pktlen = (16 * 1024) - 128; > > + dev_info->min_rx_bufsize = 1024; > Using some macros instead of hardcoded values helps to understand values. Done > > > The general usage of DEBUG_TRACE is providing backtrace log, function > enterance / exit informations. I guess, that is why it has been > controlled by different config option. > Here what you need looks like regular debugging functions, PMD_DRV_LOG / > RTE_LOG variant. I'm unclear on your request or comment. Are you suggesting that we use the dpdk versions of debug? The advantage of a local version is that they can be enabled without all the debug traces. > Can this overflow args variable? Any way to prevent possible crash? > What happens if somebody, by accident, provides a file as device > argument which is larger than 256 bytes? Code has been fixed to avoid overflow. >> > +/* >> >> > + * Although Arkville is a physical device we take advantage of the virtual >> >> > + * device initialization as a per test runtime initialization for >> >> > + * regression testing. Parameters are passed into the virtual device to >> >> > + * configure the packet generator, packet director and packet checker. >> >> > + */ >> >> Why not providing these arguments via physical device? Code has been changed to use dev arg instead of vdev args. >> +++ b/drivers/net/ark/rte_pmd_ark_version.map > s/DPDK_2.0/DPDK_17.05 Fixed.