* [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-15 13:37 ` Paolo Abeni 0 siblings, 0 replies; 15+ messages in thread From: Paolo Abeni @ 2016-06-15 13:37 UTC (permalink / raw) To: netdev Cc: Jeff Kirsher, intel-wired-lan, David S. Miller, Hannes Frederic Sowa Currently the function ixgbe_poll() returns 0 when it clean completely the rx rings, but this foul budget accounting in core code. Fix this returning the actual work done, capped to weight - 1, since the core doesn't allow to return the full budget when the driver modifies the napi status Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 088c47c..8bebd86 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget) if (!test_bit(__IXGBE_DOWN, &adapter->state)) ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector->v_idx)); - return 0; + return min(work_done, budget - 1); } /** -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-15 13:37 ` Paolo Abeni 0 siblings, 0 replies; 15+ messages in thread From: Paolo Abeni @ 2016-06-15 13:37 UTC (permalink / raw) To: intel-wired-lan Currently the function ixgbe_poll() returns 0 when it clean completely the rx rings, but this foul budget accounting in core code. Fix this returning the actual work done, capped to weight - 1, since the core doesn't allow to return the full budget when the driver modifies the napi status Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 088c47c..8bebd86 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget) if (!test_bit(__IXGBE_DOWN, &adapter->state)) ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector->v_idx)); - return 0; + return min(work_done, budget - 1); } /** -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net] ixgbe: napi_poll must return the work done 2016-06-15 13:37 ` [Intel-wired-lan] " Paolo Abeni @ 2016-06-15 15:20 ` Alexander Duyck -1 siblings, 0 replies; 15+ messages in thread From: Alexander Duyck @ 2016-06-15 15:20 UTC (permalink / raw) To: Paolo Abeni Cc: Netdev, Jeff Kirsher, intel-wired-lan, David S. Miller, Hannes Frederic Sowa On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> wrote: > Currently the function ixgbe_poll() returns 0 when it clean completely > the rx rings, but this foul budget accounting in core code. > Fix this returning the actual work done, capped to weight - 1, since > the core doesn't allow to return the full budget when the driver modifies > the napi status > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> I think the origin of reporting 0 was actually compatibility with some NAPI code floating around from before the 2.6.24 kernel. I'd be curious to know how much this is actually fouling things up. Can you point to any specific issues it was causing? If you end up having to submit a v2 for any reason it might be useful if you can provide the additional details on what actual issue it was causing. You might also want to look at the other Intel drivers, specifically ixgbevf and fm10k as I believe we have similar code in those drivers as well. Acked-by: Alexander Duyck <aduyck@mirantis.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-15 15:20 ` Alexander Duyck 0 siblings, 0 replies; 15+ messages in thread From: Alexander Duyck @ 2016-06-15 15:20 UTC (permalink / raw) To: intel-wired-lan On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> wrote: > Currently the function ixgbe_poll() returns 0 when it clean completely > the rx rings, but this foul budget accounting in core code. > Fix this returning the actual work done, capped to weight - 1, since > the core doesn't allow to return the full budget when the driver modifies > the napi status > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> I think the origin of reporting 0 was actually compatibility with some NAPI code floating around from before the 2.6.24 kernel. I'd be curious to know how much this is actually fouling things up. Can you point to any specific issues it was causing? If you end up having to submit a v2 for any reason it might be useful if you can provide the additional details on what actual issue it was causing. You might also want to look at the other Intel drivers, specifically ixgbevf and fm10k as I believe we have similar code in those drivers as well. Acked-by: Alexander Duyck <aduyck@mirantis.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] ixgbe: napi_poll must return the work done 2016-06-15 15:20 ` [Intel-wired-lan] " Alexander Duyck @ 2016-06-15 15:43 ` Paolo Abeni -1 siblings, 0 replies; 15+ messages in thread From: Paolo Abeni @ 2016-06-15 15:43 UTC (permalink / raw) To: Alexander Duyck Cc: Netdev, Jeff Kirsher, intel-wired-lan, David S. Miller, Hannes Frederic Sowa On Wed, 2016-06-15 at 08:20 -0700, Alexander Duyck wrote: > On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> wrote: > > Currently the function ixgbe_poll() returns 0 when it clean completely > > the rx rings, but this foul budget accounting in core code. > > Fix this returning the actual work done, capped to weight - 1, since > > the core doesn't allow to return the full budget when the driver modifies > > the napi status > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > I think the origin of reporting 0 was actually compatibility with some > NAPI code floating around from before the 2.6.24 kernel. > > I'd be curious to know how much this is actually fouling things up. > Can you point to any specific issues it was causing? I noticed this while instrumenting the napi poll loop for another patch. It's not easy to reproduce the bugged scenario, several NICs receiving a relevant amount of traffic on napi instances scheduled on the same softirq are needed. If any/some of them has the buggy poll() method, the napi_poll() loop may process (much) more than netdev_budget packets per invocation, possibly delaying others softirq more than needed/expected. The maxium delay will be no matter what capped to a couple of jiffies, due to the time-based loop end condition, so in the worst possible scenario (most probably not a real thing), this adds a latency of 2 jiffies - <time required to process netdev_budget packets> (~1.8ms on recent h/w with HZ==1000). > If you end up > having to submit a v2 for any reason it might be useful if you can > provide the additional details on what actual issue it was causing. > > You might also want to look at the other Intel drivers, specifically > ixgbevf and fm10k as I believe we have similar code in those drivers > as well. Thank you for the head-up. I need to get an hand on that h/w, first! Paolo > > Acked-by: Alexander Duyck <aduyck@mirantis.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-15 15:43 ` Paolo Abeni 0 siblings, 0 replies; 15+ messages in thread From: Paolo Abeni @ 2016-06-15 15:43 UTC (permalink / raw) To: intel-wired-lan On Wed, 2016-06-15 at 08:20 -0700, Alexander Duyck wrote: > On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> wrote: > > Currently the function ixgbe_poll() returns 0 when it clean completely > > the rx rings, but this foul budget accounting in core code. > > Fix this returning the actual work done, capped to weight - 1, since > > the core doesn't allow to return the full budget when the driver modifies > > the napi status > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > I think the origin of reporting 0 was actually compatibility with some > NAPI code floating around from before the 2.6.24 kernel. > > I'd be curious to know how much this is actually fouling things up. > Can you point to any specific issues it was causing? I noticed this while instrumenting the napi poll loop for another patch. It's not easy to reproduce the bugged scenario, several NICs receiving a relevant amount of traffic on napi instances scheduled on the same softirq are needed. If any/some of them has the buggy poll() method, the napi_poll() loop may process (much) more than netdev_budget packets per invocation, possibly delaying others softirq more than needed/expected. The maxium delay will be no matter what capped to a couple of jiffies, due to the time-based loop end condition, so in the worst possible scenario (most probably not a real thing), this adds a latency of 2 jiffies - <time required to process netdev_budget packets> (~1.8ms on recent h/w with HZ==1000). > If you end up > having to submit a v2 for any reason it might be useful if you can > provide the additional details on what actual issue it was causing. > > You might also want to look at the other Intel drivers, specifically > ixgbevf and fm10k as I believe we have similar code in those drivers > as well. Thank you for the head-up. I need to get an hand on that h/w, first! Paolo > > Acked-by: Alexander Duyck <aduyck@mirantis.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] ixgbe: napi_poll must return the work done 2016-06-15 13:37 ` [Intel-wired-lan] " Paolo Abeni @ 2016-06-15 16:34 ` Venkatesh Srinivas -1 siblings, 0 replies; 15+ messages in thread From: Venkatesh Srinivas @ 2016-06-15 16:34 UTC (permalink / raw) To: Paolo Abeni Cc: netdev, Jeff Kirsher, intel-wired-lan, David S. Miller, Hannes Frederic Sowa On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> wrote: > Currently the function ixgbe_poll() returns 0 when it clean completely > the rx rings, but this foul budget accounting in core code. > Fix this returning the actual work done, capped to weight - 1, since > the core doesn't allow to return the full budget when the driver modifies > the napi status > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 088c47c..8bebd86 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget) > if (!test_bit(__IXGBE_DOWN, &adapter->state)) > ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector->v_idx)); > > - return 0; > + return min(work_done, budget - 1); > } > > /** Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> The same bit of code appears in fm10k and i40e/i40evf. ixgb appears to correctly return work_done. ixgbe_poll also appears to return an (minor) incorrect work_done in another case, BTW. It divides its budget between Rx rings associated with a vector. If any ring exceeds its share of the budget, ixgbe_poll claims to have consumed the full budget, even if a full budget of frames was not received in a single pass. -- vs; ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-15 16:34 ` Venkatesh Srinivas 0 siblings, 0 replies; 15+ messages in thread From: Venkatesh Srinivas @ 2016-06-15 16:34 UTC (permalink / raw) To: intel-wired-lan On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> wrote: > Currently the function ixgbe_poll() returns 0 when it clean completely > the rx rings, but this foul budget accounting in core code. > Fix this returning the actual work done, capped to weight - 1, since > the core doesn't allow to return the full budget when the driver modifies > the napi status > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 088c47c..8bebd86 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget) > if (!test_bit(__IXGBE_DOWN, &adapter->state)) > ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector->v_idx)); > > - return 0; > + return min(work_done, budget - 1); > } > > /** Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> The same bit of code appears in fm10k and i40e/i40evf. ixgb appears to correctly return work_done. ixgbe_poll also appears to return an (minor) incorrect work_done in another case, BTW. It divides its budget between Rx rings associated with a vector. If any ring exceeds its share of the budget, ixgbe_poll claims to have consumed the full budget, even if a full budget of frames was not received in a single pass. -- vs; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] ixgbe: napi_poll must return the work done 2016-06-15 16:34 ` [Intel-wired-lan] " Venkatesh Srinivas @ 2016-06-16 17:10 ` Keller, Jacob E -1 siblings, 0 replies; 15+ messages in thread From: Keller, Jacob E @ 2016-06-16 17:10 UTC (permalink / raw) To: venkateshs, pabeni Cc: hannes, netdev, Kirsher, Jeffrey T, intel-wired-lan, davem On Wed, 2016-06-15 at 09:34 -0700, Venkatesh Srinivas wrote: > On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> > wrote: > > > > Currently the function ixgbe_poll() returns 0 when it clean > > completely > > the rx rings, but this foul budget accounting in core code. > > Fix this returning the actual work done, capped to weight - 1, > > since > > the core doesn't allow to return the full budget when the driver > > modifies > > the napi status > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > --- > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > index 088c47c..8bebd86 100644 > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int > > budget) > > if (!test_bit(__IXGBE_DOWN, &adapter->state)) > > ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector- > > >v_idx)); > > > > - return 0; > > + return min(work_done, budget - 1); > > } > > > > /** > Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> > > The same bit of code appears in fm10k and i40e/i40evf. ixgb appears > to > correctly return work_done. > > ixgbe_poll also appears to return an (minor) incorrect work_done in > another case, BTW. It divides its > budget between Rx rings associated with a vector. If any ring exceeds > its share of the budget, ixgbe_poll > claims to have consumed the full budget, even if a full budget of > frames was not received in a single > pass. > > -- vs; I can submit a patch for fm10k. Thanks, Jake ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-16 17:10 ` Keller, Jacob E 0 siblings, 0 replies; 15+ messages in thread From: Keller, Jacob E @ 2016-06-16 17:10 UTC (permalink / raw) To: intel-wired-lan On Wed, 2016-06-15 at 09:34 -0700, Venkatesh Srinivas wrote: > On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> > wrote: > > > > Currently the function ixgbe_poll() returns 0 when it clean > > completely > > the rx rings, but this foul budget accounting in core code. > > Fix this returning the actual work done, capped to weight - 1, > > since > > the core doesn't allow to return the full budget when the driver > > modifies > > the napi status > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > --- > > ?drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > > ?1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > index 088c47c..8bebd86 100644 > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int > > budget) > > ????????if (!test_bit(__IXGBE_DOWN, &adapter->state)) > > ????????????????ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector- > > >v_idx)); > > > > -???????return 0; > > +???????return min(work_done, budget - 1); > > ?} > > > > ?/** > Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> > > The same bit of code appears in fm10k and i40e/i40evf. ixgb appears > to > correctly return work_done. > > ixgbe_poll also appears to return an (minor) incorrect work_done in > another case, BTW. It divides its > budget between Rx rings associated with a vector. If any ring exceeds > its share of the budget, ixgbe_poll > claims to have consumed the full budget, even if a full budget of > frames was not received in a single > pass. > > -- vs; I can submit a patch for fm10k. Thanks, Jake ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] ixgbe: napi_poll must return the work done 2016-06-15 16:34 ` [Intel-wired-lan] " Venkatesh Srinivas @ 2016-06-16 17:40 ` Keller, Jacob E -1 siblings, 0 replies; 15+ messages in thread From: Keller, Jacob E @ 2016-06-16 17:40 UTC (permalink / raw) To: venkateshs, pabeni Cc: hannes, netdev, Kirsher, Jeffrey T, intel-wired-lan, davem On Wed, 2016-06-15 at 09:34 -0700, Venkatesh Srinivas wrote: > On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> > wrote: > > > > Currently the function ixgbe_poll() returns 0 when it clean > > completely > > the rx rings, but this foul budget accounting in core code. > > Fix this returning the actual work done, capped to weight - 1, > > since > > the core doesn't allow to return the full budget when the driver > > modifies > > the napi status > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > --- > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > index 088c47c..8bebd86 100644 > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int > > budget) > > if (!test_bit(__IXGBE_DOWN, &adapter->state)) > > ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector- > > >v_idx)); > > > > - return 0; > > + return min(work_done, budget - 1); > > } > > > > /** > Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> > > The same bit of code appears in fm10k and i40e/i40evf. ixgb appears > to > correctly return work_done. > > ixgbe_poll also appears to return an (minor) incorrect work_done in > another case, BTW. It divides its > budget between Rx rings associated with a vector. If any ring exceeds > its share of the budget, ixgbe_poll > claims to have consumed the full budget, even if a full budget of > frames was not received in a single > pass. > > -- vs; For the record, I couldn't find any documentation on this in Documentatino/networking, or as a function header. Where would be the best place to document the expectations of the napi core? I'd like to submit a patch so that future it will be easier to determine what a new driver should do (without just blindly copying from other drivers as has caused these so far). Thanks, Jake ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-16 17:40 ` Keller, Jacob E 0 siblings, 0 replies; 15+ messages in thread From: Keller, Jacob E @ 2016-06-16 17:40 UTC (permalink / raw) To: intel-wired-lan On Wed, 2016-06-15 at 09:34 -0700, Venkatesh Srinivas wrote: > On Wed, Jun 15, 2016 at 6:37 AM, Paolo Abeni <pabeni@redhat.com> > wrote: > > > > Currently the function ixgbe_poll() returns 0 when it clean > > completely > > the rx rings, but this foul budget accounting in core code. > > Fix this returning the actual work done, capped to weight - 1, > > since > > the core doesn't allow to return the full budget when the driver > > modifies > > the napi status > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > --- > > ?drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > > ?1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > index 088c47c..8bebd86 100644 > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > > @@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int > > budget) > > ????????if (!test_bit(__IXGBE_DOWN, &adapter->state)) > > ????????????????ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector- > > >v_idx)); > > > > -???????return 0; > > +???????return min(work_done, budget - 1); > > ?} > > > > ?/** > Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> > > The same bit of code appears in fm10k and i40e/i40evf. ixgb appears > to > correctly return work_done. > > ixgbe_poll also appears to return an (minor) incorrect work_done in > another case, BTW. It divides its > budget between Rx rings associated with a vector. If any ring exceeds > its share of the budget, ixgbe_poll > claims to have consumed the full budget, even if a full budget of > frames was not received in a single > pass. > > -- vs; For the record, I couldn't find any documentation on this in Documentatino/networking, or as a function header. Where would be the best place to document the expectations of the napi core? I'd like to submit a patch so that future it will be easier to determine what a new driver should do (without just blindly copying from other drivers as has caused these so far). Thanks, Jake ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] ixgbe: napi_poll must return the work done 2016-06-15 16:34 ` [Intel-wired-lan] " Venkatesh Srinivas @ 2016-06-16 17:43 ` Keller, Jacob E -1 siblings, 0 replies; 15+ messages in thread From: Keller, Jacob E @ 2016-06-16 17:43 UTC (permalink / raw) To: venkateshs, pabeni Cc: hannes, netdev, Kirsher, Jeffrey T, intel-wired-lan, davem On Wed, 2016-06-15 at 09:34 -0700, Venkatesh Srinivas wrote: > Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> > > The same bit of code appears in fm10k and i40e/i40evf. ixgb appears > to > correctly return work_done. > > ixgbe_poll also appears to return an (minor) incorrect work_done in > another case, BTW. It divides its > budget between Rx rings associated with a vector. If any ring exceeds > its share of the budget, ixgbe_poll > claims to have consumed the full budget, even if a full budget of > frames was not received in a single > pass. > So the correct return here would also be min between work_done and budget, right? ie: we should still return the total work done instead of the budget...? Thanks, Jake > -- vs; ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done @ 2016-06-16 17:43 ` Keller, Jacob E 0 siblings, 0 replies; 15+ messages in thread From: Keller, Jacob E @ 2016-06-16 17:43 UTC (permalink / raw) To: intel-wired-lan On Wed, 2016-06-15 at 09:34 -0700, Venkatesh Srinivas wrote: > Reviewed-by: Venkatesh Srinivas <venkateshs@google.com> > > The same bit of code appears in fm10k and i40e/i40evf. ixgb appears > to > correctly return work_done. > > ixgbe_poll also appears to return an?(minor) incorrect work_done in > another case, BTW. It divides its > budget between Rx rings associated with a vector. If any ring exceeds > its share of the budget, ixgbe_poll > claims to have consumed the full budget, even if a full budget of > frames was not received in a single > pass. > So the correct return here would also be min between work_done and budget, right? ie: we should still return the total work done instead of the budget...? Thanks, Jake > -- vs; ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work done 2016-06-15 13:37 ` [Intel-wired-lan] " Paolo Abeni ` (2 preceding siblings ...) (?) @ 2016-07-13 15:12 ` Bowers, AndrewX -1 siblings, 0 replies; 15+ messages in thread From: Bowers, AndrewX @ 2016-07-13 15:12 UTC (permalink / raw) To: intel-wired-lan > -----Original Message----- > From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On > Behalf Of Paolo Abeni > Sent: Wednesday, June 15, 2016 6:38 AM > To: netdev at vger.kernel.org > Cc: Hannes Frederic Sowa <hannes@redhat.com>; intel-wired- > lan at lists.osuosl.org; David S. Miller <davem@davemloft.net> > Subject: [Intel-wired-lan] [PATCH net] ixgbe: napi_poll must return the work > done > > Currently the function ixgbe_poll() returns 0 when it clean completely the rx > rings, but this foul budget accounting in core code. > Fix this returning the actual work done, capped to weight - 1, since the core > doesn't allow to return the full budget when the driver modifies the napi > status > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Tested-by: Andrew Bowers <andrewx.bowers@intel.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-07-13 15:12 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-15 13:37 [PATCH net] ixgbe: napi_poll must return the work done Paolo Abeni 2016-06-15 13:37 ` [Intel-wired-lan] " Paolo Abeni 2016-06-15 15:20 ` Alexander Duyck 2016-06-15 15:20 ` [Intel-wired-lan] " Alexander Duyck 2016-06-15 15:43 ` Paolo Abeni 2016-06-15 15:43 ` [Intel-wired-lan] " Paolo Abeni 2016-06-15 16:34 ` Venkatesh Srinivas 2016-06-15 16:34 ` [Intel-wired-lan] " Venkatesh Srinivas 2016-06-16 17:10 ` Keller, Jacob E 2016-06-16 17:10 ` [Intel-wired-lan] " Keller, Jacob E 2016-06-16 17:40 ` Keller, Jacob E 2016-06-16 17:40 ` [Intel-wired-lan] " Keller, Jacob E 2016-06-16 17:43 ` Keller, Jacob E 2016-06-16 17:43 ` [Intel-wired-lan] " Keller, Jacob E 2016-07-13 15:12 ` Bowers, AndrewX
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.