From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qg0-f67.google.com (mail-qg0-f67.google.com [209.85.192.67]) by dpdk.org (Postfix) with ESMTP id D2623AA39 for ; Tue, 1 Mar 2016 18:40:49 +0100 (CET) Received: by mail-qg0-f67.google.com with SMTP id p68so8319181qge.2 for ; Tue, 01 Mar 2016 09:40:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=1Vy2xe+/fEDYxDbW/ybGBK9H4nB7UZ6MyGLVPUlWmHI=; b=uutI0xSXuz3+FAMLB0wfWAWW/ejBJPIMDNrgkZJJZuXvgmBsA4Y+x5jlQLicQfd+Zr lgkbM/srfCBGNo/hj39XdiIB5Fh7Qa4X6+E8NHurMrGLGu1KgjEcG/NhAKQ/2myGwGxZ 06EOATqQlXF2g5/yUSYzdkA2s6bVuqE0ejx64ZuWD2E+1kGAgN3ldF8WnDPbi3zIMObH LKX1yrZFkTmlwYPAdu659Q/CvmpHtmvIn82UAkmfYovKo9rmA4tyQXMuwaj5c00pEnek 7NvC/u1MoqtDLKlIuug0JJetz9bDZF/O1dmpPsSl5MkOUplWqYPfPFOuqARdM5ZPdoMV nKHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=1Vy2xe+/fEDYxDbW/ybGBK9H4nB7UZ6MyGLVPUlWmHI=; b=UcFiQA/cKJAcXRvFgbnRny7Z9kp/mbEKlEE/DE7V4dZX1zjytUMfGXffrvARt6JdjF 1zumnCvI1brKdiSO8kg9N2gI68FUDjcAKRMQjQXMBNlznAdZFO13rIr+cYOWz0Czot77 CRh5sbRE1KYplRHpjxYAAZnkpgUzpjXZoE7a3I+9RU9BhUcSrXIcFTOyZJCBCUqcQSrV Q84N/Kc3T8VrmEkCxY7HkM5TFHyuzij9pUenXSBsGn+4MIjfibrP/QBNxWPzKpBGj06t I/+8CwKFE6sWdhomyzt8DZOjjZZAlz4qPOeF2/DU4yI6zENhL7M6TDCIfV+SZyqdaXRR FMYA== X-Gm-Message-State: AD7BkJLluomXDTA1PR/5K+xCUnyMEPEzx6xoZp675dz6xuQjgDTI8LnU3YpKbhlD8KfdvA== X-Received: by 10.140.18.114 with SMTP id 105mr27786397qge.41.1456854049305; Tue, 01 Mar 2016 09:40:49 -0800 (PST) Received: from gmail.com (pool-108-31-208-15.washdc.fios.verizon.net. [108.31.208.15]) by smtp.gmail.com with ESMTPSA id d6sm13418297qkb.13.2016.03.01.09.40.47 (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 01 Mar 2016 09:40:48 -0800 (PST) Received: by gmail.com (sSMTP sendmail emulation); Tue, 1 Mar 2016 12:40:46 -0500 Date: Tue, 1 Mar 2016 12:40:46 -0500 From: Eric Kinzie To: "Iremonger, Bernard" Message-ID: <20160301174042.GA14082@roosta.home> References: <8CEF83825BEC744B83065625E567D7C219FB2C51@IRSMSX108.ger.corp.intel.com> <1455909473-13539-1-git-send-email-ehkinzie@gmail.com> <1455909473-13539-5-git-send-email-ehkinzie@gmail.com> <56CB0709.6010407@redhat.com> <8CEF83825BEC744B83065625E567D7C219FB4852@IRSMSX108.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8CEF83825BEC744B83065625E567D7C219FB4852@IRSMSX108.ger.corp.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 4/6] bond mode 4: allow external state machine 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: Tue, 01 Mar 2016 17:40:50 -0000 On Thu Feb 25 15:22:35 +0000 2016, Iremonger, Bernard wrote: > Hi Eric, > > > > > @@ -157,6 +159,7 @@ struct rte_eth_bond_8023ad_conf { > > > uint32_t tx_period_ms; > > > uint32_t rx_marker_period_ms; > > > uint32_t update_timeout_ms; > > > + rte_eth_bond_8023ad_ext_slowrx_fn slowrx_cb; > > > }; > > > > This still is a likely an ABI break, previously discussed around here: > > http://dpdk.org/ml/archives/dev/2015-November/027321.html > > > > It might not be embedded anywhere in DPDK codebase, but there's no > > telling what others might have done with it (have an array of them, embed in > > other structs etc). > > > > Also ultimately ABI compatibility goes both ways: when the library soname > > does not change then an application is free to assume both downgrading and > > upgrading are safe. In this case, upgrading *might* be okay, downgrading > > certainly is not. So by that measure it definitely is an ABI break. > > > > [...] > > > diff --git a/drivers/net/bonding/rte_eth_bond_version.map > > > b/drivers/net/bonding/rte_eth_bond_version.map > > > index 22bd920..33d73ff 100644 > > > --- a/drivers/net/bonding/rte_eth_bond_version.map > > > +++ b/drivers/net/bonding/rte_eth_bond_version.map > > > @@ -27,3 +27,9 @@ DPDK_2.1 { > > > rte_eth_bond_free; > > > > > > } DPDK_2.0; > > > + > > > +DPDK_2.2 { > > > + rte_eth_bond_8023ad_ext_collect; > > > + rte_eth_bond_8023ad_ext_distrib; > > > + rte_eth_bond_8023ad_ext_slowtx; > > > +} DPDK_2.1; > > > > > > > These symbols are not part of DPDK 2.2, the version here is wrong. > > Technically it would not actually matter much but better not to confuse > > things unnecessarily. > > > > - Panu - > > It looks like Panu's points are valid, a V3 of this patch set which takes care of these issues will be needed. > > Patches 1/6, 5/6 and 6/6 of the patch set are bug fixes, so each patch should contain a fixes line. > Patches 2/6, 3/6 and 4/6 are a new feature, the release notes should be updated for this feature. > > Could I suggest splitting the patch set into two patch sets, a bug fix patch set and a new feature patch set. > > Regards, > > Bernard. Bernard, a v3 is on the way. I included only things that are fixes, but the patch set doesn't quite match the set of patch numbers you listed above. 1/6 (bond: use existing enslaved device queues) is an improvement, but doesn't really fix anything that was broken, so I left that out. 2/6 (bond mode 4: copy entire config structure) and 3/6 (bond mode 4: do not ignore multicast) fix bugs and are included. Eric