From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wj0-f182.google.com (mail-wj0-f182.google.com [209.85.210.182]) by dpdk.org (Postfix) with ESMTP id EA4F5282 for ; Tue, 27 Dec 2016 15:37:55 +0100 (CET) Received: by mail-wj0-f182.google.com with SMTP id tq7so82251307wjb.0 for ; Tue, 27 Dec 2016 06:37:55 -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=Ec1smLiqNotB1fBXwzcbPqy2NwasI7lDpjsiQoyF4WI=; b=HdgMQBkiZQtPtFJ8ZXJneZdhCWCOUSzBIao9+Pmk/1BzIrFP2gcDnTghUikqO8+4Sq FkcqjIDCgvLl5rHfcM4KK89QGHwIi1msCs6VuPLuyu415LDHTRBxHTMSAd7snfZEPw77 deb08tu8GwQe29FgkZxnV2Bg3IRklefDWCWu3raZj/3bPzAlX7jTYrwgMzlQSBLDjOWS +JIKkPYgJVDSPWYaR5cSHKa4ZsyaBQVoTyQHU5TEa6x8M+cHOxAfPuYS18Bp7Jc7O+Tu MFVeSJ1m4ljto3b2UUdWppSt4hlxS245uLprwz0RUfmRZVpQat9AJaB1hbA6VQHkqKRI jU9A== 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=Ec1smLiqNotB1fBXwzcbPqy2NwasI7lDpjsiQoyF4WI=; b=tUBjIcl/FmDG+4cxWSNKO3G7b1MfEnWlPaLy4oSY7Juc1Rnv4BasL9aiJOO9tiSN0y t3dvX9UxXTEbgsZ/5AsLbh+TACPnz9tb9ix82EguI5hIDT9TmnRxVoxq34BdtjJ3e/J1 PWOuxdYB4hN7waPcu6M1irmACdMkdbpvnM0conmz+S+/KgjwRAtcjFxIxsHHFVSvq3W8 P1ZEGA4d9iwTm5xqWRNxQKwJ7WkmBJYQs3GXGiv/A3CB+Q8iaE7fxb+znqviLpuOO6Eg dSHpDgLdde+BF3RMqHbvsn7fxfqdYC4JJ+4CG1KqfZdTlJBrDZxn/3mK5qSzp++aical vyIg== X-Gm-Message-State: AIkVDXIIONmiUAiYuSidq+criLZ83jc66fAeZtO4IfRgP9lqqh1Up/8/Bfp5kAVZgBGGEa2D X-Received: by 10.194.113.169 with SMTP id iz9mr34196521wjb.57.1482849475630; Tue, 27 Dec 2016 06:37:55 -0800 (PST) Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by smtp.gmail.com with ESMTPSA id l67sm56150575wmf.0.2016.12.27.06.37.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 27 Dec 2016 06:37:54 -0800 (PST) Date: Tue, 27 Dec 2016 15:37:46 +0100 From: Adrien Mazarguil To: Tiwei Bie Cc: dev@dpdk.org, wenzhuo.lu@intel.com, wei.dai@intel.com, xiao.w.wang@intel.com, olivier.matz@6wind.com, thomas.monjalon@6wind.com, konstantin.ananyev@intel.com, helin.zhang@intel.com Message-ID: <20161227143746.GC3737@6wind.com> References: <1481852611-103254-1-git-send-email-tiwei.bie@intel.com> <1482677880-117158-1-git-send-email-tiwei.bie@intel.com> <20161226151537.GD22106@6wind.com> <20161227013330.GA145018@dpdk19> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161227013330.GA145018@dpdk19> Subject: Re: [dpdk-dev] [PATCH v3 0/6] Add MACsec offload support for ixgbe 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, 27 Dec 2016 14:37:56 -0000 Hi Tiwei, On Tue, Dec 27, 2016 at 09:33:31AM +0800, Tiwei Bie wrote: > Hi Adrien, > > On Mon, Dec 26, 2016 at 04:15:37PM +0100, Adrien Mazarguil wrote: > > Hi Tiwei, > > > > On Sun, Dec 25, 2016 at 10:57:54PM +0800, Tiwei Bie wrote: > > > This patch set adds the MACsec offload support for ixgbe. > > > The testpmd is also updated to support MACsec cmds. > > > > I'm not commenting on any specific patch from this series, however I'm > > noticing this new trend of working around ethdev to add PMD-specific APIs. > > I would like to make sure it's not getting out of hand. > > > > To use the "rte_pmd_ixgbe_macsec_*()" API, applications must be linked with > > librte_pmd_ixgbe directly and have the related code under #ifdef > > RTE_LIBRTE_IXGBE_PMD like testpmd. > > > > Here we can see this ixgbe-specific API affects rte_mbuf.h and rte_ethdev.h > > (new PKT_TX_MACSEC, RTE_ETH_EVENT_MACSEC, DEV_RX_OFFLOAD_MACSEC_STRIP and > > DEV_TX_OFFLOAD_MACSEC_INSERT flags). > > > > - Shouldn't these flags have "IXGBE" somewhere in their name and/or be > > defined under #ifdef RTE_LIBRTE_IXGBE_PMD? > > > > I think those flags are very generic, simple and innocent, so we could just > define them in the normal way. And currently it seems that we lack a way to > define the PMD-specific flags for mbuf, ethdev, etc. Yes, this is probably the least intrusive approach. I'm just pointing out that in the meantime, assigned values are not available for other generic uses which is a problem unless there is a plan to make this API available globally. Missing flags and bits could also be defined directly by rte_pmd_ixgbe.h. > > - Why can't the MACsec API be defined globally, for instance won't i40e > > implement it as well someday? > > I think currently we prefer to implement some PMD features based on the > PMD-specific APIs at first to avoid the bloating of the ethdev APIs. And > when it proves to be generic (which means many people really need it and > care about it, and more drivers are really going to implement it), then > we make it as the global ethdev API. Understandable, but then unless the global API remains exactly the same including the "rte_ixgbe_macsec" prefix (which I think won't happen), applications need to be rewritten, it's not convenient, and as a result may prevent adoption due to the following cycle: - Applications wait for the API to evolve before using MACsec. - DPDK waits for applications to use the ixgbe MACsec API to improve it. In the end, flags tied to a single PMD remain allocated in the global namespace while the API is kept in this temporary state forever. I think doing the extra work to make it global from the start is worth the trouble. This step could perhaps be made easier if people agreed that struct eth_dev_ops (and friends) must be opaque to applications. > > - Why bothering with TX/RX offload capabilities if applications know the > > underlying PMD anyway? > > > > Not every NIC supported by IXGBE supports the MACsec offload. So even > if the application knows it's using IXGBE PMD, it still needs to check > whether the MACsec offload capabilities are supported. OK, I assumed they all did. > > Assuming these patches are kept as-is, I suggest we define a reserved space > > documented as such for PMD-specific flags wherever they are used. > > > > It sounds good to me. But it may involve many pieces of the lib, such > as the mbuf ol_flags, ethdev event type, ethdev offload capabilities, > and so on. So maybe it can be done as another work. Right, that was just an idea from the top of my head, we could define reserved values only when they become necessary for a PMD as in this case, e.g. instead of defining PKT_TX_MACSEC, one would define PKT_TX_RESERVED_0 which would be interpreted as MACSEC by ixgbe until this API is exposed globally. Other PMDs could implement other unrelated PMD-specific APIs through RESERVED_0 in the meantime, so we preserve the precious space we have for global APIs (especially inside mbufs). Thoughts? Best regards, -- Adrien Mazarguil 6WIND