From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id E4F1C1AEF3
 for <dev@dpdk.org>; Mon, 18 Sep 2017 13:27:27 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id 433DF20D36;
 Mon, 18 Sep 2017 07:27:27 -0400 (EDT)
Received: from frontend2 ([10.202.2.161])
 by compute1.internal (MEProxy); Mon, 18 Sep 2017 07:27:27 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 cc:content-transfer-encoding:content-type:date:from:in-reply-to
 :message-id:mime-version:references:subject:to:x-me-sender
 :x-me-sender:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=M7i/IrFFYeIZsC0
 sKCrIzDlc2cXAOWwRhyLi1aqGscw=; b=MA6WSUiaUAcyCelVBFHplxBmRqXZsjc
 bkf02Bp10naa0n6dqLV2FOVHXyDJaytt7DOWmgXgL+BM8yS7kzHPh6RhJtmSDPlW
 NQhUzljJTwjX5iy5pi1hzCEBF+lOVWQTif43HhQmhN11gHp6mwAPElu7Xo4Vk84k
 MCZ9XVHA04qk=
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-sender:x-me-sender:x-sasl-enc:x-sasl-enc; s=
 fm1; bh=M7i/IrFFYeIZsC0sKCrIzDlc2cXAOWwRhyLi1aqGscw=; b=p8wnId38
 FZhC94sFxha5GwkaE4jfDAbonZQncyevuOAkcth8stmHRpCgM6F4DYnDkEDBgmgw
 +HmHi4igFLwEje0OS8j6ZfAM1ogGDgVjP16o0y6XdKkHm8h2JpNkwyY4WrnUoVyp
 SrwoB3DouG5wCtfZvHc34UTDq51eNbDI/BE6FnFRAT50E4fRJ1WQ9uTDU8MpGaXY
 I4N+If7zLb+1WJnDvt6Be/h5o5uAcpx7BY6eCdiEfVItrPy0R91cNi4gDYrEurB4
 eqi2yDnx8pJRP0RU82MMfA5uefoZk2UvWIkes1iFfclw6gVGx3ZWkvFqcxnC9/Bk
 3ApMskcQLl1ciA==
X-ME-Sender: <xms:n62_WUc17KL1PIICvqEIy0SCNaZwF_fGslOjxBazv4WZyWt5Xvexbw>
X-Sasl-enc: jxl3Fd0cQHHV4E2/2kG8VLUsQFzPvNpDJewPc1g5wGK6 1505734046
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id E994824335;
 Mon, 18 Sep 2017 07:27:26 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Bruce Richardson <bruce.richardson@intel.com>, "Ananyev,
 Konstantin" <konstantin.ananyev@intel.com>
Cc: "stephen@networkplumber.org" <stephen@networkplumber.org>, dev@dpdk.org,
 Shahaf Shuler <shahafs@mellanox.com>
Date: Mon, 18 Sep 2017 13:27:26 +0200
Message-ID: <1709802.VBkrf5mcYs@xps>
In-Reply-To: <20170918110417.GA15516@bricha3-MOBL3.ger.corp.intel.com>
References: <cover.1504508374.git.shahafs@mellanox.com>
 <2601191342CEEE43887BDE71AB9772584F24BFBF@irsmsx105.ger.corp.intel.com>
 <20170918110417.GA15516@bricha3-MOBL3.ger.corp.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the new
	offloads API
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 18 Sep 2017 11:27:28 -0000

18/09/2017 13:04, Bruce Richardson:
> On Mon, Sep 18, 2017 at 11:57:03AM +0100, Ananyev, Konstantin wrote:
> > From: Richardson, Bruce
> > > On Thu, Sep 14, 2017 at 10:02:26AM +0200, Thomas Monjalon wrote:
> > > > 13/09/2017 23:42, Ananyev, Konstantin:
> > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > 13/09/2017 14:56, Ananyev, Konstantin:
> > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > Konstantin, I would like your opinion about the proposal below.
> > > > > > It is about making on the fly configuration more generic.
> > > > > > You say it is possible to configure VLAN on the fly,
> > > > > > and I think we should make it possible for other offload features.
> > > > >
> > > > > It would be a good thing, but I don't think it is possible for all offloads.
> > > > > For some of them you still have to stop the queue(port) first.
> > > > >
> > > > > Also I am not sure what exactly do you propose?
> > > > > Is that something like that:
> > > > > - wipe existing offload bitfileds from rte_eth_rxmode (already done by Shahaf)
> > > > > - Instead of uint64_t offloads inside both  rte_eth_rxmode and te_eth_rxconf
> > > > >   Introduce new functions:
> > > > >
> > > > > int rte_eth_set_port_rx_offload(portid, uint64_t offload_mask);
> > > > > int rte_eth_set_queue_rx_offload(portid, queueid, uint64_t offload_mask);
> > > Would be useful to have a valid mask here, to indicate what bits to use.
> > > That way, you can adjust one bit without worrying about what other bits
> > > you may change in the process. There are probably apps out there that
> > > just want to toggle a single bit on, and off, at runtime while ignoring
> > > others.
> > > Alternatively, we can have set/unset functions which enable/disable
> > > offloads, based on the mask.
> > 
> > My thought was  that people would do:
> > 
> > uint64_t offload = rte_eth_get_port_rx_offload(port);
> > offload |= RX_OFFLOAD_X;
> > offload &= ~RX_OFFLOAD_Y;
> > rte_eth_set_port_rx_offload(port, offload);
> > 
> > In that case, I think we don't really need a mask.
> > 
> Sure, that can work, I'm not concerned either way.
> 
> Overall, I think my slight preference would be to have set/unset,
> enable/disable functions to make it clear what is happening, rather than
> having to worry about the complete set each time.
> 
> uint64_t rte_eth_port_rx_offload_enable(port_id, offload_mask)
> uint64_t rte_eth_port_rx_offload_disable(port_id, offload_mask)
> 
> each returning the bits failing (or bits changed if you like, but I prefer
> bits failing as return value, since it means 0 == no_error).

I think we need both: "get" functions + "mask" parameters in "set" functions.