* [dpdk-dev] [PATCH] net/failsafe: add default Tx mbuf fast free capability @ 2018-10-12 11:36 Andrew Rybchenko 2018-12-21 12:12 ` [dpdk-dev] " Ferruh Yigit 0 siblings, 1 reply; 11+ messages in thread From: Andrew Rybchenko @ 2018-10-12 11:36 UTC (permalink / raw) To: Gaëtan Rivet; +Cc: dev, Ivan Malov From: Ivan Malov <ivan.malov@oktetlabs.ru> This capability is reported when supported by the current emitting sub-device. Failsafe PMD itself does not excercise fast free logic. Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> --- doc/guides/nics/features/failsafe.ini | 1 + drivers/net/failsafe/failsafe_ops.c | 1 + 2 files changed, 2 insertions(+) diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini index e3c4c08f2..b6f3dcee6 100644 --- a/doc/guides/nics/features/failsafe.ini +++ b/doc/guides/nics/features/failsafe.ini @@ -7,6 +7,7 @@ Link status = Y Link status event = Y Rx interrupt = Y +Fast mbuf free = Y Queue start/stop = Y Runtime Rx queue setup = Y Runtime Tx queue setup = Y diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 7f8bcd4c6..e3add404b 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { DEV_RX_OFFLOAD_SECURITY, .tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS | + DEV_TX_OFFLOAD_MBUF_FAST_FREE | DEV_TX_OFFLOAD_IPV4_CKSUM | DEV_TX_OFFLOAD_UDP_CKSUM | DEV_TX_OFFLOAD_TCP_CKSUM | -- 2.17.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-10-12 11:36 [dpdk-dev] [PATCH] net/failsafe: add default Tx mbuf fast free capability Andrew Rybchenko @ 2018-12-21 12:12 ` Ferruh Yigit 2018-12-21 12:28 ` Andrew Rybchenko 2018-12-21 15:09 ` Ferruh Yigit 0 siblings, 2 replies; 11+ messages in thread From: Ferruh Yigit @ 2018-12-21 12:12 UTC (permalink / raw) To: Andrew Rybchenko, Gaëtan Rivet; +Cc: dev, Ivan Malov On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: > From: Ivan Malov <ivan.malov@oktetlabs.ru> > > This capability is reported when supported by the current emitting > sub-device. Failsafe PMD itself does not excercise fast free logic. I think overlay device capability reporting already discussed a few times, the question is if an overlay devices should claim a feature when it depends on underlay devices? Given that no ack/review given to the patch, I am updating it as rejected. > > Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > --- > doc/guides/nics/features/failsafe.ini | 1 + > drivers/net/failsafe/failsafe_ops.c | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini > index e3c4c08f2..b6f3dcee6 100644 > --- a/doc/guides/nics/features/failsafe.ini > +++ b/doc/guides/nics/features/failsafe.ini > @@ -7,6 +7,7 @@ > Link status = Y > Link status event = Y > Rx interrupt = Y > +Fast mbuf free = Y > Queue start/stop = Y > Runtime Rx queue setup = Y > Runtime Tx queue setup = Y > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c > index 7f8bcd4c6..e3add404b 100644 > --- a/drivers/net/failsafe/failsafe_ops.c > +++ b/drivers/net/failsafe/failsafe_ops.c > @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { > DEV_RX_OFFLOAD_SECURITY, > .tx_offload_capa = > DEV_TX_OFFLOAD_MULTI_SEGS | > + DEV_TX_OFFLOAD_MBUF_FAST_FREE | > DEV_TX_OFFLOAD_IPV4_CKSUM | > DEV_TX_OFFLOAD_UDP_CKSUM | > DEV_TX_OFFLOAD_TCP_CKSUM | > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 12:12 ` [dpdk-dev] " Ferruh Yigit @ 2018-12-21 12:28 ` Andrew Rybchenko 2018-12-21 12:43 ` Ferruh Yigit 2018-12-21 15:09 ` Ferruh Yigit 1 sibling, 1 reply; 11+ messages in thread From: Andrew Rybchenko @ 2018-12-21 12:28 UTC (permalink / raw) To: Ferruh Yigit, Gaëtan Rivet; +Cc: dev, Ivan Malov On 12/21/18 3:12 PM, Ferruh Yigit wrote: > On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: >> From: Ivan Malov <ivan.malov@oktetlabs.ru> >> >> This capability is reported when supported by the current emitting >> sub-device. Failsafe PMD itself does not excercise fast free logic. > I think overlay device capability reporting already discussed a few times, the > question is if an overlay devices should claim a feature when it depends on > underlay devices? The capability may be reported by the failsafe since it is transparent from fast free logic point of view. > Given that no ack/review given to the patch, I am updating it as rejected. Is it a new policy? I thought that it was vice versa before. >> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >> --- >> doc/guides/nics/features/failsafe.ini | 1 + >> drivers/net/failsafe/failsafe_ops.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini >> index e3c4c08f2..b6f3dcee6 100644 >> --- a/doc/guides/nics/features/failsafe.ini >> +++ b/doc/guides/nics/features/failsafe.ini >> @@ -7,6 +7,7 @@ >> Link status = Y >> Link status event = Y >> Rx interrupt = Y >> +Fast mbuf free = Y >> Queue start/stop = Y >> Runtime Rx queue setup = Y >> Runtime Tx queue setup = Y >> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c >> index 7f8bcd4c6..e3add404b 100644 >> --- a/drivers/net/failsafe/failsafe_ops.c >> +++ b/drivers/net/failsafe/failsafe_ops.c >> @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { >> DEV_RX_OFFLOAD_SECURITY, >> .tx_offload_capa = >> DEV_TX_OFFLOAD_MULTI_SEGS | >> + DEV_TX_OFFLOAD_MBUF_FAST_FREE | >> DEV_TX_OFFLOAD_IPV4_CKSUM | >> DEV_TX_OFFLOAD_UDP_CKSUM | >> DEV_TX_OFFLOAD_TCP_CKSUM | >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 12:28 ` Andrew Rybchenko @ 2018-12-21 12:43 ` Ferruh Yigit 2018-12-21 12:52 ` Andrew Rybchenko 0 siblings, 1 reply; 11+ messages in thread From: Ferruh Yigit @ 2018-12-21 12:43 UTC (permalink / raw) To: Andrew Rybchenko, Gaëtan Rivet; +Cc: dev, Ivan Malov On 12/21/2018 12:28 PM, Andrew Rybchenko wrote: > On 12/21/18 3:12 PM, Ferruh Yigit wrote: >> On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: >>> From: Ivan Malov <ivan.malov@oktetlabs.ru> >>> >>> This capability is reported when supported by the current emitting >>> sub-device. Failsafe PMD itself does not excercise fast free logic. >> I think overlay device capability reporting already discussed a few times, the >> question is if an overlay devices should claim a feature when it depends on >> underlay devices? > > The capability may be reported by the failsafe since it is transparent from > fast free logic point of view. Why it is transparent? If one of the underlying device doesn't support/claim this feature, application can't use this feature with failsafe, isn't it? > >> Given that no ack/review given to the patch, I am updating it as rejected. > > Is it a new policy? I thought that it was vice versa before. Hi Andrew, Yes policy is other-way around indeed, when there is no comment at all default behavior is accept, but please take above paragraph as my comment to the patch. And I was thinking it is a little controversial and there is no support to have it, so lets don't get it. What do you think? > >>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> >>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>> --- >>> doc/guides/nics/features/failsafe.ini | 1 + >>> drivers/net/failsafe/failsafe_ops.c | 1 + >>> 2 files changed, 2 insertions(+) >>> >>> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini >>> index e3c4c08f2..b6f3dcee6 100644 >>> --- a/doc/guides/nics/features/failsafe.ini >>> +++ b/doc/guides/nics/features/failsafe.ini >>> @@ -7,6 +7,7 @@ >>> Link status = Y >>> Link status event = Y >>> Rx interrupt = Y >>> +Fast mbuf free = Y >>> Queue start/stop = Y >>> Runtime Rx queue setup = Y >>> Runtime Tx queue setup = Y >>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c >>> index 7f8bcd4c6..e3add404b 100644 >>> --- a/drivers/net/failsafe/failsafe_ops.c >>> +++ b/drivers/net/failsafe/failsafe_ops.c >>> @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { >>> DEV_RX_OFFLOAD_SECURITY, >>> .tx_offload_capa = >>> DEV_TX_OFFLOAD_MULTI_SEGS | >>> + DEV_TX_OFFLOAD_MBUF_FAST_FREE | >>> DEV_TX_OFFLOAD_IPV4_CKSUM | >>> DEV_TX_OFFLOAD_UDP_CKSUM | >>> DEV_TX_OFFLOAD_TCP_CKSUM | >>> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 12:43 ` Ferruh Yigit @ 2018-12-21 12:52 ` Andrew Rybchenko 2018-12-21 13:16 ` Gaëtan Rivet 2018-12-21 13:31 ` Ferruh Yigit 0 siblings, 2 replies; 11+ messages in thread From: Andrew Rybchenko @ 2018-12-21 12:52 UTC (permalink / raw) To: Ferruh Yigit, Gaëtan Rivet; +Cc: dev, Ivan Malov Hi Ferruh, On 12/21/18 3:43 PM, Ferruh Yigit wrote: > On 12/21/2018 12:28 PM, Andrew Rybchenko wrote: >> On 12/21/18 3:12 PM, Ferruh Yigit wrote: >>> On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: >>>> From: Ivan Malov <ivan.malov@oktetlabs.ru> >>>> >>>> This capability is reported when supported by the current emitting >>>> sub-device. Failsafe PMD itself does not excercise fast free logic. >>> I think overlay device capability reporting already discussed a few times, the >>> question is if an overlay devices should claim a feature when it depends on >>> underlay devices? >> The capability may be reported by the failsafe since it is transparent from >> fast free logic point of view. > Why it is transparent? If one of the underlying device doesn't support/claim > this feature, application can't use this feature with failsafe, isn't it? tx_offload_capa in failsafe is a mask to apply on sub-device capabilities. So, if the capability is not supported by any sub-device it will not be reported. As well if there is the capability bit in the mask, it will not be reported regardless sub-devices capabilities. The description for the patch above tries to explain it - it looks like not that successful. >>> Given that no ack/review given to the patch, I am updating it as rejected. >> Is it a new policy? I thought that it was vice versa before. > Hi Andrew, > > Yes policy is other-way around indeed, when there is no comment at all default > behavior is accept, but please take above paragraph as my comment to the patch. Got it. > And I was thinking it is a little controversial and there is no support to have > it, so lets don't get it. What do you think? I see you motivation. >>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> >>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>>> --- >>>> doc/guides/nics/features/failsafe.ini | 1 + >>>> drivers/net/failsafe/failsafe_ops.c | 1 + >>>> 2 files changed, 2 insertions(+) >>>> >>>> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini >>>> index e3c4c08f2..b6f3dcee6 100644 >>>> --- a/doc/guides/nics/features/failsafe.ini >>>> +++ b/doc/guides/nics/features/failsafe.ini >>>> @@ -7,6 +7,7 @@ >>>> Link status = Y >>>> Link status event = Y >>>> Rx interrupt = Y >>>> +Fast mbuf free = Y >>>> Queue start/stop = Y >>>> Runtime Rx queue setup = Y >>>> Runtime Tx queue setup = Y >>>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c >>>> index 7f8bcd4c6..e3add404b 100644 >>>> --- a/drivers/net/failsafe/failsafe_ops.c >>>> +++ b/drivers/net/failsafe/failsafe_ops.c >>>> @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { >>>> DEV_RX_OFFLOAD_SECURITY, >>>> .tx_offload_capa = >>>> DEV_TX_OFFLOAD_MULTI_SEGS | >>>> + DEV_TX_OFFLOAD_MBUF_FAST_FREE | >>>> DEV_TX_OFFLOAD_IPV4_CKSUM | >>>> DEV_TX_OFFLOAD_UDP_CKSUM | >>>> DEV_TX_OFFLOAD_TCP_CKSUM | >>>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 12:52 ` Andrew Rybchenko @ 2018-12-21 13:16 ` Gaëtan Rivet 2018-12-21 13:59 ` Ferruh Yigit 2018-12-21 18:10 ` Stephen Hemminger 2018-12-21 13:31 ` Ferruh Yigit 1 sibling, 2 replies; 11+ messages in thread From: Gaëtan Rivet @ 2018-12-21 13:16 UTC (permalink / raw) To: Andrew Rybchenko; +Cc: Ferruh Yigit, dev, Ivan Malov Hi Ferruh, Andrew, On Fri, Dec 21, 2018 at 03:52:07PM +0300, Andrew Rybchenko wrote: > Hi Ferruh, > > On 12/21/18 3:43 PM, Ferruh Yigit wrote: > > On 12/21/2018 12:28 PM, Andrew Rybchenko wrote: > > > On 12/21/18 3:12 PM, Ferruh Yigit wrote: > > > > On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: > > > > > From: Ivan Malov <ivan.malov@oktetlabs.ru> > > > > > > > > > > This capability is reported when supported by the current emitting > > > > > sub-device. Failsafe PMD itself does not excercise fast free logic. > > > > I think overlay device capability reporting already discussed a few times, the > > > > question is if an overlay devices should claim a feature when it depends on > > > > underlay devices? > > > The capability may be reported by the failsafe since it is transparent from > > > fast free logic point of view. > > Why it is transparent? If one of the underlying device doesn't support/claim > > this feature, application can't use this feature with failsafe, isn't it? If a VF is forbidden by its PF from adding MAC addresses, the driver should still advertize support for "Unicast MAC filter" right? This is the same here. Fail-safe needs to forward configurations items to its sub-device for a feature to work. Sometimes, the hardware won't be sufficient. But the fail-safe itself still has the parts needed (even if it is only a flag to add to a feature list). It is necessary, at the fail-safe level, to be able to describe the current feature set. This is what the feature list is for. There are important caveats to consider however, which is inherent to using the fail-safe. It does not mean those features should be removed from the fail-safe feature list. > > tx_offload_capa in failsafe is a mask to apply on sub-device capabilities. > So, if the capability is not supported by any sub-device it will not be > reported. > As well if there is the capability bit in the mask, it will not be reported > regardless > sub-devices capabilities. The description for the patch above tries to > explain it - > it looks like not that successful. > > > > > Given that no ack/review given to the patch, I am updating it as rejected. > > > Is it a new policy? I thought that it was vice versa before. > > Hi Andrew, > > > > Yes policy is other-way around indeed, when there is no comment at all default > > behavior is accept, but please take above paragraph as my comment to the patch. > > Got it. > > > And I was thinking it is a little controversial and there is no support to have > > it, so lets don't get it. What do you think? > > I see you motivation. > > > > > > Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> > > > > > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > > > > > --- > > > > > doc/guides/nics/features/failsafe.ini | 1 + > > > > > drivers/net/failsafe/failsafe_ops.c | 1 + > > > > > 2 files changed, 2 insertions(+) > > > > > > > > > > diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini > > > > > index e3c4c08f2..b6f3dcee6 100644 > > > > > --- a/doc/guides/nics/features/failsafe.ini > > > > > +++ b/doc/guides/nics/features/failsafe.ini > > > > > @@ -7,6 +7,7 @@ > > > > > Link status = Y > > > > > Link status event = Y > > > > > Rx interrupt = Y > > > > > +Fast mbuf free = Y > > > > > Queue start/stop = Y > > > > > Runtime Rx queue setup = Y > > > > > Runtime Tx queue setup = Y > > > > > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c > > > > > index 7f8bcd4c6..e3add404b 100644 > > > > > --- a/drivers/net/failsafe/failsafe_ops.c > > > > > +++ b/drivers/net/failsafe/failsafe_ops.c > > > > > @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { > > > > > DEV_RX_OFFLOAD_SECURITY, > > > > > .tx_offload_capa = > > > > > DEV_TX_OFFLOAD_MULTI_SEGS | > > > > > + DEV_TX_OFFLOAD_MBUF_FAST_FREE | > > > > > DEV_TX_OFFLOAD_IPV4_CKSUM | > > > > > DEV_TX_OFFLOAD_UDP_CKSUM | > > > > > DEV_TX_OFFLOAD_TCP_CKSUM | > > > > > > -- Gaëtan Rivet 6WIND ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 13:16 ` Gaëtan Rivet @ 2018-12-21 13:59 ` Ferruh Yigit 2018-12-21 14:44 ` Gaëtan Rivet 2018-12-21 18:10 ` Stephen Hemminger 1 sibling, 1 reply; 11+ messages in thread From: Ferruh Yigit @ 2018-12-21 13:59 UTC (permalink / raw) To: Gaëtan Rivet, Andrew Rybchenko; +Cc: dev, Ivan Malov On 12/21/2018 1:16 PM, Gaëtan Rivet wrote: > Hi Ferruh, Andrew, > > On Fri, Dec 21, 2018 at 03:52:07PM +0300, Andrew Rybchenko wrote: >> Hi Ferruh, >> >> On 12/21/18 3:43 PM, Ferruh Yigit wrote: >>> On 12/21/2018 12:28 PM, Andrew Rybchenko wrote: >>>> On 12/21/18 3:12 PM, Ferruh Yigit wrote: >>>>> On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: >>>>>> From: Ivan Malov <ivan.malov@oktetlabs.ru> >>>>>> >>>>>> This capability is reported when supported by the current emitting >>>>>> sub-device. Failsafe PMD itself does not excercise fast free logic. >>>>> I think overlay device capability reporting already discussed a few times, the >>>>> question is if an overlay devices should claim a feature when it depends on >>>>> underlay devices? >>>> The capability may be reported by the failsafe since it is transparent from >>>> fast free logic point of view. >>> Why it is transparent? If one of the underlying device doesn't support/claim >>> this feature, application can't use this feature with failsafe, isn't it? > > If a VF is forbidden by its PF from adding MAC addresses, the driver > should still advertize support for "Unicast MAC filter" right? > > This is the same here. Fail-safe needs to forward configurations items > to its sub-device for a feature to work. Sometimes, the hardware won't > be sufficient. But the fail-safe itself still has the parts needed (even > if it is only a flag to add to a feature list). I see your point, also I think it may be misleading for an overlay device to announce a feature which is completely depends underlay devices. Anyway this may be a nuance. I am OK with the change after Andrew's explanation, and as far as I understand you are OK too, may I add your explicit ack to the patch? > > It is necessary, at the fail-safe level, to be able to describe the > current feature set. This is what the feature list is for. There are > important caveats to consider however, which is inherent to using the > fail-safe. > > It does not mean those features should be removed from the fail-safe > feature list. > >> >> tx_offload_capa in failsafe is a mask to apply on sub-device capabilities. >> So, if the capability is not supported by any sub-device it will not be >> reported. >> As well if there is the capability bit in the mask, it will not be reported >> regardless >> sub-devices capabilities. The description for the patch above tries to >> explain it - >> it looks like not that successful. >> >>>>> Given that no ack/review given to the patch, I am updating it as rejected. >>>> Is it a new policy? I thought that it was vice versa before. >>> Hi Andrew, >>> >>> Yes policy is other-way around indeed, when there is no comment at all default >>> behavior is accept, but please take above paragraph as my comment to the patch. >> >> Got it. >> >>> And I was thinking it is a little controversial and there is no support to have >>> it, so lets don't get it. What do you think? >> >> I see you motivation. >> >>>>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> >>>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>>>>> --- >>>>>> doc/guides/nics/features/failsafe.ini | 1 + >>>>>> drivers/net/failsafe/failsafe_ops.c | 1 + >>>>>> 2 files changed, 2 insertions(+) >>>>>> >>>>>> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini >>>>>> index e3c4c08f2..b6f3dcee6 100644 >>>>>> --- a/doc/guides/nics/features/failsafe.ini >>>>>> +++ b/doc/guides/nics/features/failsafe.ini >>>>>> @@ -7,6 +7,7 @@ >>>>>> Link status = Y >>>>>> Link status event = Y >>>>>> Rx interrupt = Y >>>>>> +Fast mbuf free = Y >>>>>> Queue start/stop = Y >>>>>> Runtime Rx queue setup = Y >>>>>> Runtime Tx queue setup = Y >>>>>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c >>>>>> index 7f8bcd4c6..e3add404b 100644 >>>>>> --- a/drivers/net/failsafe/failsafe_ops.c >>>>>> +++ b/drivers/net/failsafe/failsafe_ops.c >>>>>> @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { >>>>>> DEV_RX_OFFLOAD_SECURITY, >>>>>> .tx_offload_capa = >>>>>> DEV_TX_OFFLOAD_MULTI_SEGS | >>>>>> + DEV_TX_OFFLOAD_MBUF_FAST_FREE | >>>>>> DEV_TX_OFFLOAD_IPV4_CKSUM | >>>>>> DEV_TX_OFFLOAD_UDP_CKSUM | >>>>>> DEV_TX_OFFLOAD_TCP_CKSUM | >>>>>> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 13:59 ` Ferruh Yigit @ 2018-12-21 14:44 ` Gaëtan Rivet 0 siblings, 0 replies; 11+ messages in thread From: Gaëtan Rivet @ 2018-12-21 14:44 UTC (permalink / raw) To: Ferruh Yigit; +Cc: Andrew Rybchenko, dev, Ivan Malov On Fri, Dec 21, 2018 at 01:59:20PM +0000, Ferruh Yigit wrote: > On 12/21/2018 1:16 PM, Gaëtan Rivet wrote: > > Hi Ferruh, Andrew, > > > > On Fri, Dec 21, 2018 at 03:52:07PM +0300, Andrew Rybchenko wrote: > >> Hi Ferruh, > >> > >> On 12/21/18 3:43 PM, Ferruh Yigit wrote: > >>> On 12/21/2018 12:28 PM, Andrew Rybchenko wrote: > >>>> On 12/21/18 3:12 PM, Ferruh Yigit wrote: > >>>>> On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: > >>>>>> From: Ivan Malov <ivan.malov@oktetlabs.ru> > >>>>>> > >>>>>> This capability is reported when supported by the current emitting > >>>>>> sub-device. Failsafe PMD itself does not excercise fast free logic. > >>>>> I think overlay device capability reporting already discussed a few times, the > >>>>> question is if an overlay devices should claim a feature when it depends on > >>>>> underlay devices? > >>>> The capability may be reported by the failsafe since it is transparent from > >>>> fast free logic point of view. > >>> Why it is transparent? If one of the underlying device doesn't support/claim > >>> this feature, application can't use this feature with failsafe, isn't it? > > > > If a VF is forbidden by its PF from adding MAC addresses, the driver > > should still advertize support for "Unicast MAC filter" right? > > > > This is the same here. Fail-safe needs to forward configurations items > > to its sub-device for a feature to work. Sometimes, the hardware won't > > be sufficient. But the fail-safe itself still has the parts needed (even > > if it is only a flag to add to a feature list). > > I see your point, also I think it may be misleading for an overlay device to > announce a feature which is completely depends underlay devices. Anyway this may > be a nuance. > > I am OK with the change after Andrew's explanation, and as far as I understand > you are OK too, may I add your explicit ack to the patch? > Yes the patch is ok for me, thank you Ferruh. > > > > It is necessary, at the fail-safe level, to be able to describe the > > current feature set. This is what the feature list is for. There are > > important caveats to consider however, which is inherent to using the > > fail-safe. > > > > It does not mean those features should be removed from the fail-safe > > feature list. > > > >> > >> tx_offload_capa in failsafe is a mask to apply on sub-device capabilities. > >> So, if the capability is not supported by any sub-device it will not be > >> reported. > >> As well if there is the capability bit in the mask, it will not be reported > >> regardless > >> sub-devices capabilities. The description for the patch above tries to > >> explain it - > >> it looks like not that successful. > >> > >>>>> Given that no ack/review given to the patch, I am updating it as rejected. > >>>> Is it a new policy? I thought that it was vice versa before. > >>> Hi Andrew, > >>> > >>> Yes policy is other-way around indeed, when there is no comment at all default > >>> behavior is accept, but please take above paragraph as my comment to the patch. > >> > >> Got it. > >> > >>> And I was thinking it is a little controversial and there is no support to have > >>> it, so lets don't get it. What do you think? > >> > >> I see you motivation. > >> > >>>>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> > >>>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > >>>>>> --- > >>>>>> doc/guides/nics/features/failsafe.ini | 1 + > >>>>>> drivers/net/failsafe/failsafe_ops.c | 1 + > >>>>>> 2 files changed, 2 insertions(+) > >>>>>> > >>>>>> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini > >>>>>> index e3c4c08f2..b6f3dcee6 100644 > >>>>>> --- a/doc/guides/nics/features/failsafe.ini > >>>>>> +++ b/doc/guides/nics/features/failsafe.ini > >>>>>> @@ -7,6 +7,7 @@ > >>>>>> Link status = Y > >>>>>> Link status event = Y > >>>>>> Rx interrupt = Y > >>>>>> +Fast mbuf free = Y > >>>>>> Queue start/stop = Y > >>>>>> Runtime Rx queue setup = Y > >>>>>> Runtime Tx queue setup = Y > >>>>>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c > >>>>>> index 7f8bcd4c6..e3add404b 100644 > >>>>>> --- a/drivers/net/failsafe/failsafe_ops.c > >>>>>> +++ b/drivers/net/failsafe/failsafe_ops.c > >>>>>> @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { > >>>>>> DEV_RX_OFFLOAD_SECURITY, > >>>>>> .tx_offload_capa = > >>>>>> DEV_TX_OFFLOAD_MULTI_SEGS | > >>>>>> + DEV_TX_OFFLOAD_MBUF_FAST_FREE | > >>>>>> DEV_TX_OFFLOAD_IPV4_CKSUM | > >>>>>> DEV_TX_OFFLOAD_UDP_CKSUM | > >>>>>> DEV_TX_OFFLOAD_TCP_CKSUM | > >>>>>> > >> > > > -- Gaëtan Rivet 6WIND ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 13:16 ` Gaëtan Rivet 2018-12-21 13:59 ` Ferruh Yigit @ 2018-12-21 18:10 ` Stephen Hemminger 1 sibling, 0 replies; 11+ messages in thread From: Stephen Hemminger @ 2018-12-21 18:10 UTC (permalink / raw) To: Gaëtan Rivet; +Cc: Andrew Rybchenko, Ferruh Yigit, dev, Ivan Malov On Fri, 21 Dec 2018 14:16:46 +0100 Gaëtan Rivet <gaetan.rivet@6wind.com> wrote: > If a VF is forbidden by its PF from adding MAC addresses, the driver > should still advertize support for "Unicast MAC filter" right? Not unless both VF and other path support multiple MAC addresses. Since Hyper-V/Azure is limited to a single MAC address by the vswitch it would not be appropriate there. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 12:52 ` Andrew Rybchenko 2018-12-21 13:16 ` Gaëtan Rivet @ 2018-12-21 13:31 ` Ferruh Yigit 1 sibling, 0 replies; 11+ messages in thread From: Ferruh Yigit @ 2018-12-21 13:31 UTC (permalink / raw) To: Andrew Rybchenko, Gaëtan Rivet; +Cc: dev, Ivan Malov On 12/21/2018 12:52 PM, Andrew Rybchenko wrote: > Hi Ferruh, > > On 12/21/18 3:43 PM, Ferruh Yigit wrote: >> On 12/21/2018 12:28 PM, Andrew Rybchenko wrote: >>> On 12/21/18 3:12 PM, Ferruh Yigit wrote: >>>> On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: >>>>> From: Ivan Malov <ivan.malov@oktetlabs.ru> >>>>> >>>>> This capability is reported when supported by the current emitting >>>>> sub-device. Failsafe PMD itself does not excercise fast free logic. >>>> I think overlay device capability reporting already discussed a few times, the >>>> question is if an overlay devices should claim a feature when it depends on >>>> underlay devices? >>> The capability may be reported by the failsafe since it is transparent from >>> fast free logic point of view. >> Why it is transparent? If one of the underlying device doesn't support/claim >> this feature, application can't use this feature with failsafe, isn't it? > > tx_offload_capa in failsafe is a mask to apply on sub-device capabilities. I missed this one, I see why it is transparent. Why failsafe doesn't set a full tx_offload_capa MASK but maintain a list? > So, if the capability is not supported by any sub-device it will not be > reported. > As well if there is the capability bit in the mask, it will not be > reported regardless > sub-devices capabilities. The description for the patch above tries to > explain it - > it looks like not that successful. > >>>> Given that no ack/review given to the patch, I am updating it as rejected. >>> Is it a new policy? I thought that it was vice versa before. >> Hi Andrew, >> >> Yes policy is other-way around indeed, when there is no comment at all default >> behavior is accept, but please take above paragraph as my comment to the patch. > > Got it. > >> And I was thinking it is a little controversial and there is no support to have >> it, so lets don't get it. What do you think? > > I see you motivation. > >>>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> >>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>>>> --- >>>>> doc/guides/nics/features/failsafe.ini | 1 + >>>>> drivers/net/failsafe/failsafe_ops.c | 1 + >>>>> 2 files changed, 2 insertions(+) >>>>> >>>>> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini >>>>> index e3c4c08f2..b6f3dcee6 100644 >>>>> --- a/doc/guides/nics/features/failsafe.ini >>>>> +++ b/doc/guides/nics/features/failsafe.ini >>>>> @@ -7,6 +7,7 @@ >>>>> Link status = Y >>>>> Link status event = Y >>>>> Rx interrupt = Y >>>>> +Fast mbuf free = Y >>>>> Queue start/stop = Y >>>>> Runtime Rx queue setup = Y >>>>> Runtime Tx queue setup = Y >>>>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c >>>>> index 7f8bcd4c6..e3add404b 100644 >>>>> --- a/drivers/net/failsafe/failsafe_ops.c >>>>> +++ b/drivers/net/failsafe/failsafe_ops.c >>>>> @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = { >>>>> DEV_RX_OFFLOAD_SECURITY, >>>>> .tx_offload_capa = >>>>> DEV_TX_OFFLOAD_MULTI_SEGS | >>>>> + DEV_TX_OFFLOAD_MBUF_FAST_FREE | >>>>> DEV_TX_OFFLOAD_IPV4_CKSUM | >>>>> DEV_TX_OFFLOAD_UDP_CKSUM | >>>>> DEV_TX_OFFLOAD_TCP_CKSUM | >>>>> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability 2018-12-21 12:12 ` [dpdk-dev] " Ferruh Yigit 2018-12-21 12:28 ` Andrew Rybchenko @ 2018-12-21 15:09 ` Ferruh Yigit 1 sibling, 0 replies; 11+ messages in thread From: Ferruh Yigit @ 2018-12-21 15:09 UTC (permalink / raw) To: Andrew Rybchenko, Gaëtan Rivet; +Cc: dev, Ivan Malov On 12/21/2018 12:12 PM, Ferruh Yigit wrote: > On 10/12/2018 12:36 PM, Andrew Rybchenko wrote: >> From: Ivan Malov <ivan.malov@oktetlabs.ru> >> >> This capability is reported when supported by the current emitting >> sub-device. Failsafe PMD itself does not excercise fast free logic. >> >> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-12-21 18:10 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-12 11:36 [dpdk-dev] [PATCH] net/failsafe: add default Tx mbuf fast free capability Andrew Rybchenko 2018-12-21 12:12 ` [dpdk-dev] " Ferruh Yigit 2018-12-21 12:28 ` Andrew Rybchenko 2018-12-21 12:43 ` Ferruh Yigit 2018-12-21 12:52 ` Andrew Rybchenko 2018-12-21 13:16 ` Gaëtan Rivet 2018-12-21 13:59 ` Ferruh Yigit 2018-12-21 14:44 ` Gaëtan Rivet 2018-12-21 18:10 ` Stephen Hemminger 2018-12-21 13:31 ` Ferruh Yigit 2018-12-21 15:09 ` Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).