-----Original Message----- > Date: Mon, 27 Nov 2017 07:03:54 +0000 > From: Shahaf Shuler <shah...@mellanox.com> > To: Andrew Rybchenko <arybche...@solarflare.com>, "dev@dpdk.org" > <dev@dpdk.org> > Subject: Re: [dpdk-dev] [PATCH 01/39] examples/l2fwd: convert to new ethdev > offloads API > > Monday, November 27, 2017 8:35 AM, Andrew Rybchenko: > > Yes this is right. Not exposing the CRC offload flag means the device don’t > support CRC strip toggling, however it does not explicitly say if device > always strip/not. > I guess device that has such limitation should specify it on the “Limitation” > section of the PMD guide. > > If it is interpreted in such way it sounds like loss of functionality. > Don't think it is a good way to rely on documentation here. It should > be more reliable way. PMD still can check if offload is not enabled and > complain, but there is no way to say that it is strictly required. > As I understand similar things are covered with so-called fixed offloads > in Linux. > > Can you elaborate which functionality is being lost here? > If your suggestion is for the PMD to force the CRC STRIP offload in case it > is not supporting *not* to strip CRC then I am OK with that. > > Yes it is. > With the new Tx offloads API the application can choose the Tx offloads it > wants to use according to its needs. > For l2fwd case – it doesn’t use any of them. Any default txq flag the PMD set > there is irrelevant. > What I tried to do is not to preserve the entire old behavior rather to > evolve the examples/applications while keeping the same functionality (i.e. > the offloads which the application use are set, the rest are not). > > That's true for checksum and VLAN offloads, but false for fast-free. > As I understand l2fwd and many other examples meet fast-free > requirements and if PMD supports it, it should be used since it will > show better performance results. > > I agree about the Fast free offload. However IMO such optimization can be > introduced on other series which further more optimize the performance of > such applications, what do you think?
If Fast free offload is meeting the l2fwd or l3fwd example application requirements, Why not to add it now? > > --Shahaf > > From: Andrew Rybchenko [mailto:arybche...@solarflare.com] > Sent: Monday, November 27, 2017 8:35 AM > To: Shahaf Shuler <shah...@mellanox.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 01/39] examples/l2fwd: convert to new ethdev > offloads API > > On 11/26/2017 10:41 AM, Shahaf Shuler wrote: > > >>+ .ignore_offload_bitfield = 1, > > >>+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP, > > > >It is not directly related to the patch. > >May be I miss something, but it looks like there is no way to say that > >"I always strip CRC and cannot preserve it". > > Yes this is right. Not exposing the CRC offload flag means the device don’t > support CRC strip toggling, however it does not explicitly say if device > always strip/not. > I guess device that has such limitation should specify it on the “Limitation” > section of the PMD guide. > > If it is interpreted in such way it sounds like loss of functionality. > Don't think it is a good way to rely on documentation here. It should > be more reliable way. PMD still can check if offload is not enabled and > complain, but there is no way to say that it is strictly required. > As I understand similar things are covered with so-called fixed offloads > in Linux. > > > > >>+ txq_conf = dev_info.default_txconf; > > >>+ txq_conf.txq_flags = ETH_TXQ_FLAGS_IGNORE; > > >>+ txq_conf.offloads = port_conf.txmode.offloads; > > > >It looks like it is not 100% equivalent. As far as I can see dev_info get > >does > >not convert txq_flags to offloads in default_txconf and in any case > >txq_conf.offloads > >are overwritten here. So, if PMD provides default txq_flags, it is lost. > >If it is intentionally, it should be highlighted and explained. > > Yes it is. > With the new Tx offloads API the application can choose the Tx offloads it > wants to use according to its needs. > For l2fwd case – it doesn’t use any of them. Any default txq flag the PMD set > there is irrelevant. > What I tried to do is not to preserve the entire old behavior rather to > evolve the examples/applications while keeping the same functionality (i.e. > the offloads which the application use are set, the rest are not). > > That's true for checksum and VLAN offloads, but false for fast-free. > As I understand l2fwd and many other examples meet fast-free > requirements and if PMD supports it, it should be used since it will > show better performance results. > > > Moreover – it is a wrong approach, IMO, that the PMD set default offloads > flags to the application. It has no knowledge to do so. I think this > mechanism was initially created since the Tx offloads were all set by > default, so it provided a mean to have good OOB configuration. Now when all > offloads are set, I am not sure such API is needed anymore. > Will be happy to hear more opinion on that. > > I agree that blindly using PMD default offloads is a wrong approach.
