From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f47.google.com (mail-wg0-f47.google.com [74.125.82.47]) by dpdk.org (Postfix) with ESMTP id 22501682E for ; Thu, 28 Aug 2014 10:46:37 +0200 (CEST) Received: by mail-wg0-f47.google.com with SMTP id z12so415802wgg.6 for ; Thu, 28 Aug 2014 01:50:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=Z4Kdwu+nRn1Doc7ybzi7vxPl1D3CS8XXwiyi229cl+M=; b=NG8+hH8o27WJp2jC5J+4qFVUjwNodvbexU3vg8n9NcEts86/+73J9F6dtayvGXhtuK f1bcCT4guOXa5Xj+3vVOHI+3p5t4YKY4lzkOl7PWIjW3elDxvbmuOX1yALV5cmsUiAgN YmNXYWAnorqT42QUk9CKY4onIbn2L/CCCkm+Ssbmt2O2//n2n9eo9hS6ax6O2DvqOPtH lTnuVqHSmrXj0jr9XM3iCMkS74TXIPEZrhfUX63onx4LWacnuBMFq3He7T6G+qWuS9I5 6LJFFFP/2cE4NMeCLFH5zFUP15nrFW9Moikx8V21EGv4M8RnCrrFLsD2RZzOgkqSKgTP VWkQ== X-Gm-Message-State: ALoCoQmYWBbSrdNCGWTKa+OEql06Fi9QURvw24PJVoaPuUkP6TEFbOtTDOPD2kATo6LdjZfEJwxe X-Received: by 10.194.71.11 with SMTP id q11mr3147672wju.33.1409215843315; Thu, 28 Aug 2014 01:50:43 -0700 (PDT) Received: from xps13.localnet (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id pm6sm8099956wjb.36.2014.08.28.01.50.42 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Aug 2014 01:50:42 -0700 (PDT) From: Thomas Monjalon To: "Wu, Jingjing" Date: Thu, 28 Aug 2014 10:50:37 +0200 Message-ID: <6406194.6E43HquPHH@xps13> Organization: 6WIND User-Agent: KMail/4.13.3 (Linux/3.15.8-1-ARCH; KDE/4.13.3; x86_64; ; ) In-Reply-To: <9BB6961774997848B5B42BEC655768F8ADBF4E@SHSMSX104.ccr.corp.intel.com> References: <1409105634-29980-1-git-send-email-jingjing.wu@intel.com> <8438692.KHYKcsiDRz@xps13> <9BB6961774997848B5B42BEC655768F8ADBF4E@SHSMSX104.ccr.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Aug 2014 08:46:37 -0000 2014-08-28 03:51, Wu, Jingjing: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > 2014-08-27 10:13, Jingjing Wu: > > > add structure definition to construct programming packet. > > > > What is a "programming packet"? > > For Fortville, we need to set a flow director filter by sending a > packet which contains the input set values through the queue > belonging to flow director. OK. To be more clear, some detailed explanations are required in the commit log. Please try to be very descriptive. You can think comments like this: - if comments are absolutely needed to understand the code, you should put comments in the code (or write the code differently) - if some feature context can help for the review, you should explain context and design in the commit log > > > +#ifdef RTE_LIBRTE_I40E_PMD > > > + "i40e_flow_director_filter (port_id) (add|del)" > > > + " flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)" > > > + " flexwords (flexwords_value) (drop|fwd)" > > > + " queue (queue_id) fd_id (fd_id_value)\n" > > > + " Add/Del a IP type flow director filter for i40e NIC.\n\n" > > > + > > > + "i40e_flow_director_filter (port_id) (add|del)" > > > + " flow (udp4|tcp4|udp6|tcp6)" > > > + " src (src_ip_address) (src_port)" > > > + " dst (dst_ip_address) (dst_port)" > > > + " flexwords (flexwords_value) (drop|fwd)" > > > + " queue (queue_id) fd_id (fd_id_value)\n" > > > + " Add/Del a UDP/TCP type flow director filter for i40e NIC.\n\n" > > > + > > > + "i40e_flush_flow_diretor (port_id)\n" > > > + " Flush all flow director entries of a device on i40e NIC.\n\n" > > > +#endif /* RTE_LIBRTE_I40E_PMD */ > > > > I'd really like to stop seeing this kind of thing. > > We cannot add some ifdef for each PMD in generic code. > > > > I stopped reading after that. > > > > Sorry, I don't want to be rude but my feeling is that adding such feature > > with global picture in mind is not easy. I know you want to offer all i40e > > capabilities but you should think at future evolutions and how other drivers > > will be integrated with yours. > > > > Sorry to make you feel uncomfortable for such code. Just as you say, > I want to offer more i40e capabilities. I will rework code in testpmd. Thanks -- Thomas