From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id 5B79F1B0F9 for ; Wed, 3 Apr 2019 11:14:35 +0200 (CEST) Received: by mail-wm1-f65.google.com with SMTP id y197so7232575wmd.0 for ; Wed, 03 Apr 2019 02:14:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=PDGbMSkmggJIi5rp16daeKsoQZg80uJER7ceI1Z0XOI=; b=e23SqaX6I1pTcA/XDKcXEuUdsgHvC23zWjmoji0G64hGw5LcxJxykok7Cw0fiupfpK cLztWDFtfFlJYjfk1kFjfpgK0mEdTwRZ5Y5UuCZZ9cP6U+WEvA11CQEG1LTHXvucGyuM DV24Jbjv6GmptNgXcIJ6gG0wxqqZWbb8QxqzUzjtNbvUwXus6cn/3t/cBoPq7+pkI9wQ NpUMj6mO1O6ni6tnAssru+OwvXlbfMetf9kmHPffYFdRvDGQgIvTfdkucD/YZPdg+KVY YETsBoZN/NrL5pYXrfY/2NsSpehCTdVhF4WsOBZa7A+dTR1QGc52ZyqFVhNQgQ+JecJl qPPg== 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=PDGbMSkmggJIi5rp16daeKsoQZg80uJER7ceI1Z0XOI=; b=eiZAUBHoXxWsXu+xmhjvpv3rCdm1k+B8Y4va5DKJM3srQXlino8xDIRTaMQIVQM8IV 4hnppKsPzwWvMmqamMo45LUzPWNTwGCMeZZmU4JwU6ejaL4HATOAifp9iglWM7k9ltp/ 5dAyP4HvsQMmD0lEVzJs1E6RdxCFargAP1SftxrlY9YFxKlKgdgOFZilAgvNasC1TrDs Ksl9AbNb/n/pR4ptuhpOMSK7Wp3dhwYcqE2xdCEEGndNny356iCLpilkQl1u7WIxjnLs fB3o+8k9i9T6AZYlsCfGZ5MoAnxdpB61gdiq8v4R/m7RTkmLgRqbZA5JbhlZ91wM2HlH ZYGw== X-Gm-Message-State: APjAAAXk9RWYkfW5rlfo5uznD0/usu5QWtN902I2EHVkvXKNZX7umqPF Jqi1Vfx1jpvAV61c1WmnIX0rFw== X-Google-Smtp-Source: APXvYqxuzYBeSNlmHxwijQqwZtERwWX3/M4M74onnkQYsu0RNDwceTtxEwD6AFbhkhK4U2rCk1J0qg== X-Received: by 2002:a1c:1aca:: with SMTP id a193mr1367350wma.40.1554282875121; Wed, 03 Apr 2019 02:14:35 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id b3sm17709353wrx.57.2019.04.03.02.14.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Apr 2019 02:14:34 -0700 (PDT) Date: Wed, 3 Apr 2019 11:14:32 +0200 From: Adrien Mazarguil To: Dekel Peled Cc: wenzhuo.lu@intel.com, jingjing.wu@intel.com, bernard.iremonger@intel.com, yskoh@mellanox.com, shahafs@mellanox.com, dev@dpdk.org, orika@mellanox.com Message-ID: <20190403091432.GP4889@6wind.com> References: <1553177917-43297-1-git-send-email-dekelp@mellanox.com> <1554218001-62012-2-git-send-email-dekelp@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1554218001-62012-2-git-send-email-dekelp@mellanox.com> Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: add actions to modify TCP header fields 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: Wed, 03 Apr 2019 09:14:35 -0000 Hi Dekel, On Tue, Apr 02, 2019 at 06:13:19PM +0300, Dekel Peled wrote: > Add actions: > - INC_TCP_SEQ - Increase sequence number in the outermost TCP header. > - DEC_TCP_SEQ - Decrease sequence number in the outermost TCP header. > - INC_TCP_ACK - Increase acknowledgment number in the outermost TCP > header. > - DEC_TCP_ACK - Decrease acknowledgment number in the outermost TCP > header. > > Original work by Xiaoyu Min. > > Signed-off-by: Dekel Peled > +Action: ``INC_TCP_SEQ`` > +^^^^^^^^^^^^^^^^^^^^^^^ > + > +Increase sequence number in the outermost TCP header. > + > +If this action is used without a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern item, > +behavior is unspecified, depending on PMD implementation. I still don't agree with the wording as it implies one must combine this action with the TCP pattern item or else, while one should simply ensure the presence of TCP traffic somehow. This may be done by a prior filtering rule. So here's a generic suggestion which could be used with pretty much all modifying actions (other actions have the same problem and will have to be fixed as well eventually): Using this action on non-matching traffic results in undefined behavior. This comment applies to all instances in this patch. > +/** > + * @warning > + * @b EXPERIMENTAL: this structure may change without prior notice > + * > + * RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ > + * > + * Increase/Decrease outermost TCP sequence number > + */ > +struct rte_flow_action_modify_tcp_seq { > + rte_be32_t value; /**< Value to increase/decrease by. */ > +}; > + > +/** > + * @warning > + * @b EXPERIMENTAL: this structure may change without prior notice > + * > + * RTE_FLOW_ACTION_TYPE_INC_TCP_ACK > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK > + * > + * Increase/Decrease outermost TCP acknowledgment number. > + */ > +struct rte_flow_action_modify_tcp_ack { > + rte_be32_t value; /**< Value to increase/decrease by. */ > +}; Thanks for adding experimental tags and comments, however you didn't reply anything about using a single action, or at least a single structure for add/sub/set? I'd like to hear your thoughts. -- Adrien Mazarguil 6WIND From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id C7C83A0679 for ; Wed, 3 Apr 2019 11:14:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 071671B0FC; Wed, 3 Apr 2019 11:14:37 +0200 (CEST) Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id 5B79F1B0F9 for ; Wed, 3 Apr 2019 11:14:35 +0200 (CEST) Received: by mail-wm1-f65.google.com with SMTP id y197so7232575wmd.0 for ; Wed, 03 Apr 2019 02:14:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=PDGbMSkmggJIi5rp16daeKsoQZg80uJER7ceI1Z0XOI=; b=e23SqaX6I1pTcA/XDKcXEuUdsgHvC23zWjmoji0G64hGw5LcxJxykok7Cw0fiupfpK cLztWDFtfFlJYjfk1kFjfpgK0mEdTwRZ5Y5UuCZZ9cP6U+WEvA11CQEG1LTHXvucGyuM DV24Jbjv6GmptNgXcIJ6gG0wxqqZWbb8QxqzUzjtNbvUwXus6cn/3t/cBoPq7+pkI9wQ NpUMj6mO1O6ni6tnAssru+OwvXlbfMetf9kmHPffYFdRvDGQgIvTfdkucD/YZPdg+KVY YETsBoZN/NrL5pYXrfY/2NsSpehCTdVhF4WsOBZa7A+dTR1QGc52ZyqFVhNQgQ+JecJl qPPg== 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=PDGbMSkmggJIi5rp16daeKsoQZg80uJER7ceI1Z0XOI=; b=eiZAUBHoXxWsXu+xmhjvpv3rCdm1k+B8Y4va5DKJM3srQXlino8xDIRTaMQIVQM8IV 4hnppKsPzwWvMmqamMo45LUzPWNTwGCMeZZmU4JwU6ejaL4HATOAifp9iglWM7k9ltp/ 5dAyP4HvsQMmD0lEVzJs1E6RdxCFargAP1SftxrlY9YFxKlKgdgOFZilAgvNasC1TrDs Ksl9AbNb/n/pR4ptuhpOMSK7Wp3dhwYcqE2xdCEEGndNny356iCLpilkQl1u7WIxjnLs fB3o+8k9i9T6AZYlsCfGZ5MoAnxdpB61gdiq8v4R/m7RTkmLgRqbZA5JbhlZ91wM2HlH ZYGw== X-Gm-Message-State: APjAAAXk9RWYkfW5rlfo5uznD0/usu5QWtN902I2EHVkvXKNZX7umqPF Jqi1Vfx1jpvAV61c1WmnIX0rFw== X-Google-Smtp-Source: APXvYqxuzYBeSNlmHxwijQqwZtERwWX3/M4M74onnkQYsu0RNDwceTtxEwD6AFbhkhK4U2rCk1J0qg== X-Received: by 2002:a1c:1aca:: with SMTP id a193mr1367350wma.40.1554282875121; Wed, 03 Apr 2019 02:14:35 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id b3sm17709353wrx.57.2019.04.03.02.14.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Apr 2019 02:14:34 -0700 (PDT) Date: Wed, 3 Apr 2019 11:14:32 +0200 From: Adrien Mazarguil To: Dekel Peled Cc: wenzhuo.lu@intel.com, jingjing.wu@intel.com, bernard.iremonger@intel.com, yskoh@mellanox.com, shahafs@mellanox.com, dev@dpdk.org, orika@mellanox.com Message-ID: <20190403091432.GP4889@6wind.com> References: <1553177917-43297-1-git-send-email-dekelp@mellanox.com> <1554218001-62012-2-git-send-email-dekelp@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <1554218001-62012-2-git-send-email-dekelp@mellanox.com> Subject: Re: [dpdk-dev] [PATCH v2 1/3] ethdev: add actions to modify TCP header fields 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190403091432.Ch_fYZzl2enKyM350EaG_uB_3IVQK2G3wlN_Y7oD9xc@z> Hi Dekel, On Tue, Apr 02, 2019 at 06:13:19PM +0300, Dekel Peled wrote: > Add actions: > - INC_TCP_SEQ - Increase sequence number in the outermost TCP header. > - DEC_TCP_SEQ - Decrease sequence number in the outermost TCP header. > - INC_TCP_ACK - Increase acknowledgment number in the outermost TCP > header. > - DEC_TCP_ACK - Decrease acknowledgment number in the outermost TCP > header. > > Original work by Xiaoyu Min. > > Signed-off-by: Dekel Peled > +Action: ``INC_TCP_SEQ`` > +^^^^^^^^^^^^^^^^^^^^^^^ > + > +Increase sequence number in the outermost TCP header. > + > +If this action is used without a valid RTE_FLOW_ITEM_TYPE_TCP flow pattern item, > +behavior is unspecified, depending on PMD implementation. I still don't agree with the wording as it implies one must combine this action with the TCP pattern item or else, while one should simply ensure the presence of TCP traffic somehow. This may be done by a prior filtering rule. So here's a generic suggestion which could be used with pretty much all modifying actions (other actions have the same problem and will have to be fixed as well eventually): Using this action on non-matching traffic results in undefined behavior. This comment applies to all instances in this patch. > +/** > + * @warning > + * @b EXPERIMENTAL: this structure may change without prior notice > + * > + * RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ > + * > + * Increase/Decrease outermost TCP sequence number > + */ > +struct rte_flow_action_modify_tcp_seq { > + rte_be32_t value; /**< Value to increase/decrease by. */ > +}; > + > +/** > + * @warning > + * @b EXPERIMENTAL: this structure may change without prior notice > + * > + * RTE_FLOW_ACTION_TYPE_INC_TCP_ACK > + * RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK > + * > + * Increase/Decrease outermost TCP acknowledgment number. > + */ > +struct rte_flow_action_modify_tcp_ack { > + rte_be32_t value; /**< Value to increase/decrease by. */ > +}; Thanks for adding experimental tags and comments, however you didn't reply anything about using a single action, or at least a single structure for add/sub/set? I'd like to hear your thoughts. -- Adrien Mazarguil 6WIND