Received-SPF: pass (google.com: domain of linux-kernel+bounces-44338-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <92958c7b-7e5f-4e25-819f-4e52f9ffcf7b@linux.intel.com>
Date: Tue, 30 Jan 2024 10:59:40 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [Intel-wired-lan] [PATCH net-next RESENT v3] ethtool: ice:
 Support for RSS settings to GTP from ethtool
To: takeru hayasaka <hayatake396@gmail.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>,
 Tony Nguyen <anthony.l.nguyen@intel.com>,
 "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org,
 vladimir.oltean@nxp.com, linux-kernel@vger.kernel.org, laforge@gnumonks.org,
 intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
 mailhol.vincent@wanadoo.fr
References: <20240127140747.905552-1-hayatake396@gmail.com>
 <154f979e-a335-461b-b72e-5e9c54fe940c@linux.intel.com>
 <CADFiAcJShbgBLXdVgs1vK1jqDFopkRcw-se4b4h0V3Yd60xLVw@mail.gmail.com>
Content-Language: en-US
From: Marcin Szycik <marcin.szycik@linux.intel.com>
In-Reply-To: <CADFiAcJShbgBLXdVgs1vK1jqDFopkRcw-se4b4h0V3Yd60xLVw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 30.01.2024 07:39, takeru hayasaka wrote:
> Hi Marcin-san
> Thanks for your review!
> 
>> Do I understand correctly that all gtpu* include TEID? Maybe write it here.
> Yes, that's correct.
> 
>> It would be nice to see a link to the patch that added GTP and 'e' flag support
> to ethtool itself ("ethtool: add support for rx-flow-hash gtp").
> I will send you the link.
> The one I sent earlier was outdated, so I've updated it to match this patch.
> https://lore.kernel.org/netdev/20240130053742.946517-1-hayatake396@gmail.com/
> 
>> gtpc(4|6) doesn't include TEID, so what is its purpose?
> In GTPC communication, there is no TEID in the CSR (Create Session Request).
> Therefore, there are cases of GTPC that do not include TEID.

The way I understand it now, this patch (and the ethtool one) adds hashing on
TEID field in GTP* headers. So I wanted to ask why do we have a case (gtpc(4|6))
that doesn't include TEID? Do we hash on other fields in this header?

> 
>> s/TEID(4byte)/TEID (4bytes)/
>> Also, I think two newlines should remain here.
> I will correct the TEID notation in the next patch!

Thanks,
Marcin

---8<---