Thursday, March 15, 2018 12:41 AM, Ferruh Yigit:
> On 3/14/2018 5:49 AM, Shahaf Shuler wrote:
> > Tuesday, March 13, 2018 1:57 PM, Ferruh Yigit:
> >>>
> >>> Again - the application should follow the API which currently
> >>> dictates how
> >> to set port offload. It is not depends on the rx_queue_offloads
> capabilities.
> >>> For example, PMD which don't support queue offloads can still have
> >> verification for the API that each port offload is set also on the
> >> queue offloads.
> >>
> >> I am not agree with this part, why to dictate application to set
> >> queue offloads if it already knows device doesn't support queue specific
> offloads?
> >
> > I agree we can make a small change in the API to not force the application
> to set the port offloads in the queue configuration. It makes sense.
> > The change will be:
> > "port offloads should be set on the port configuration. Queue offloads
> should be set on the queue configuration"
>
> I am OK to this one, this is more reasonable for devices that support only
> port
> level offloads.
>
> This looks like same as option #2 mentioned in the previous mails.
>
> >
> >>
> >> In some of the existing PMD patches, to switch to new offloading API,
> >> PMD sets [rt]x_queue_offload_capa as same as [rt]x_offload_capa,
> >
> > Well this is just wrong. Unless those PMDs support all the offloads in a
> queue level.
> >
> > The logic is "every queue offload can be counted as port offload", because
> such offload can be set on each and every queue.
> > The other way around is not correct, port offload cannot be counted as
> queue offload.
> >
> > So if such PMDs has offloads which are supported only on the port level
> they cannot be declared as queue offloads.
>
> Thanks for confirming, it would be great if you can help on the PMD new
> offload API patch reviews, to catch these kind of issues.
Sure, have me Cc in the patches so It can pass through my mailbox filters.
>
> >
> >
> >> in that case
> >> application can't know if queue specific offloads are supported or
> >> not and application may try to set queue offloads, this forces PMD to
> verify them.
> >>
> >> You confirmed [rt]x_queue_offload_capa is the way for application to
> >> know if device supports queue specific offloads or not. If these
> >> values always set to [rt]x_offload_capa, application losts this capability.
> >>
> >> Instead:
> >> - PMD that doesn't support queue specific offloads should set
> >> [rt]x_queue_offload_capa to 0
> >> - When [rt]x_queue_offload_capa is 0, application should be free to
> >> set queue offloads whatever it wants
> >
> > I don't agree, when queue_offload_capa is 0 the expected behavior from
> application is not to set any offload (if we do the change in the API that you
> are pushing to).
> > PMDs can verify it or not, but if capability is not set the application
> > should
> not set the offload. This is how the API should be defined.
>
> OK for this one.
>
> >
> >> - When [rt]x_queue_offload_capa is 0, PMD should be free to verify
> >> queue offloads but most probably shouldn't verify them since we don't
> >> know what application will send.
> >>
> >> - When [rt]x_queue_offload_capa is != 0, applications should set
> >> queue offloads at least "[rt]x_queue_offload = [rt]x_offload"
> >
> > If we do the change you are pushing it is not needed.
> > Application will set the port offload in the port configuration, and the
> queue offload in the queue configuration.
> > No need to make special treatment based on the offloads_capa.
>
> Right.
>
> >
> >> - When [rt]x_queue_offload_capa is != 0, PMD should verify the queue
> >> offloads
> >>
>
>
> Back to initial question J, is tap supports queue level offloads?
> If not it shouldn't be reporting or checking queue offloads.
>
>
> Although it will be changed after above suggested change in API, I think
> check in existing tap queue_setup, also same in mlx5, is wrong.
>
> tap_rxq_are_offloads_valid(struct rte_eth_dev *dev, uint64_t offloads) {
>
> uint64_t port_offloads = dev->data->dev_conf.rxmode.offloads;
> uint64_t queue_supp_offloads = tap_rx_offload_get_queue_capa();
> uint64_t port_supp_offloads = tap_rx_offload_get_port_capa();
>
>
> <...>
> if ((port_offloads ^ offloads) & port_supp_offloads)
> return false;
> return true;
>
> }
>
>
> take the example:
> port_supp_offloads = 11111
> port_offloads = 111
> queue_supp_offloads = 1111
> offloads = 1111
>
> (port_offloads ^ offloads) & port_supp_offloads = 1000 Which will return
> false.
>
> This only works if "port_offloads == offloads" which is practically only
> supporting port level offloads.
For mlx5, the port_supp_offloads is internal function which returns **only**
the pure port offloads (the port offloads in dev_info are
rx_offload_get_queue_capa() | rx_offload_get_port_capa())
That is, offload cannot be in both port and queue offload. So the scenario
above is not feasible.
