[dpdk-dev] [PATCH] eal: bus scan and probe never fail

2017-08-12 Thread Shreyansh Jain
Bus scan is responsible for finding devices over *all* buses.
Some of these buses might not be able to scan but that should
not prevent other buses to be scanned.

Same is the case for probing. It is possible that some devices which
were scanned didn't have a specific driver. That should not prevent
other buses from being probed.

Signed-off-by: Shreyansh Jain 

---
Until now, this decision was left onto author of bus specific scan and
probe function. But, that is incorrect.
---
 lib/librte_eal/common/eal_common_bus.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_bus.c 
b/lib/librte_eal/common/eal_common_bus.c
index 08bec2d..58e1084 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -73,11 +73,9 @@ rte_bus_scan(void)
 
TAILQ_FOREACH(bus, &rte_bus_list, next) {
ret = bus->scan();
-   if (ret) {
+   if (ret)
RTE_LOG(ERR, EAL, "Scan for (%s) bus failed.\n",
bus->name);
-   return ret;
-   }
}
 
return 0;
@@ -97,20 +95,16 @@ rte_bus_probe(void)
}
 
ret = bus->probe();
-   if (ret) {
+   if (ret)
RTE_LOG(ERR, EAL, "Bus (%s) probe failed.\n",
bus->name);
-   return ret;
-   }
}
 
if (vbus) {
ret = vbus->probe();
-   if (ret) {
+   if (ret)
RTE_LOG(ERR, EAL, "Bus (%s) probe failed.\n",
vbus->name);
-   return ret;
-   }
}
 
return 0;
-- 
2.9.3



Re: [dpdk-dev] Announcement of SSE requirement change in dpdk

2017-08-12 Thread Neil Horman
On Fri, Aug 11, 2017 at 09:29:24PM +0100, Bruce Richardson wrote:
> On Wed, Aug 09, 2017 at 04:21:32PM -0400, Neil Horman wrote:
> > Can anyone point out to me when and where the change to require SSE4.2 was
> > dicussed?  The first I saw of it was when the commit to the release notes 
> > went
> > in on August 3, and I can find no prior mention of it, save for the patches 
> > that
> > went in separately in the prior weeks.
> > 
> > Neil
> > 
> There was no real widespread discussion of it, if that's what you are
> looking for. I made the proposal via patch, and it was reviewed and
> acked by a number of folks, with nobody raising any objections at the
I had a feeling that was the case, and yes, that does concern me somewhat.  In
this particular case I think its ok, because I can't really imagine anyone using
older atom processors, but I think it could become problematic in the future If
that support line moves too far into territory in which theres downstream
support issues (with things like OVS or other tree-external applications)

> time. Possibly it was a change that should have been more widely
> publicised ahead of time, but I'm not sure what form that publicization
> should have taken, since all tech discussion happens on the dev mailing
> list anyway.
> Not that I'm planning any similar changes, but for the future, what do
> you think the process for changes like this should be - and what changes
> would classify for it? If we have a process problem, let's try and fix
> it.
> 

I don't rightly know, to be honest.  DPDK is a little unique in this situation,
since user libraries are built to just access the lowest common denominator of a
given arch.  And in many ways, so is the kernel.  I'm open to suggestions, but I
think so some sort of plan would be a good idea.  These are just off the top of
my head, and likely have drawbacks, but just to get some conversation started:

1) Use extendend ISA instructions opportunistically
By this I mean  to say, we could implement an alternatives system,
simmilar to what we have in the kernel, which can do dynamic instruction
replacement based on a run time test.  For example, you can write two versions
of a function, one which impements its method with sse4 and another version
which does the same thing using core isa instructions).  If sse4 is available at
runtime, the sse4 variant is mapped in, else the other version is.
This is something we sort of talked about before, and while theres been
general support in its philosophy, its the sort of thing that takes alot of
work, and it is only used in those cases where you know you can use the
acceleration.

2) Limit where you introduce hardware deprecation
Perhaps hardware deprecation can be announced in the same way ABI
deprecation is, and then introduced at a later date (I would make an opening
argument for the next LTS release).  Using the LTS release as a deprecation
point is nice because it lets downstream consumers standardize on a release
without having to worry about hardware support going away.

Just my $0.02.  food for thought
Neil

> Regards,
> /Bruce.
> 


Re: [dpdk-dev] [PATCH] net/i40e: fix flow control watermark mismatch

2017-08-12 Thread Zhang, Qi Z
Hi Kevin:

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Thursday, August 10, 2017 11:16 PM
> To: Zhang, Qi Z ; Wu, Jingjing 
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix flow control watermark
> mismatch
> 
> On 08/10/2017 11:48 AM, Qi Zhang wrote:
> > Flow control watermark is not read out correctly, that may cause an
> > application who not intend to change watermark but does change it with
> > a rte_eth_dev_flow_ctrl_set call right after
> > rte_eth_dev_flow_ctrl_get.
> >
> > Fixes: f53577f06925 ("i40e: support flow control")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Qi Zhang 
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 19 +--
> >  1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 71cb7d3..8b008c4 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -86,12 +86,6 @@
> >  /* Flow control default timer */
> >  #define I40E_DEFAULT_PAUSE_TIME 0xU
> >
> > -/* Flow control default high water */ -#define
> > I40E_DEFAULT_HIGH_WATER (0x1C40/1024)
> > -
> > -/* Flow control default low water */
> > -#define I40E_DEFAULT_LOW_WATER  (0x1A40/1024)
> > -
> >  /* Flow control enable fwd bit */
> >  #define I40E_PRTMAC_FWD_CTRL   0x0001
> >
> > @@ -101,6 +95,12 @@
> >  /* Kilobytes shift */
> >  #define I40E_KILOSHIFT 10
> >
> > +/* Flow control default high water */ #define
> I40E_DEFAULT_HIGH_WATER
> > +(0xF2000 >> I40E_KILOSHIFT)
> > +
> > +/* Flow control default low water */
> > +#define I40E_DEFAULT_LOW_WATER  (0xF2000 >> I40E_KILOSHIFT)
> > +
> 
> I'm not sure these are needed anymore. Would seem if you want a correct
> value prior to flow_ctl_get() being called, the registers should also be read
> during init. Is there a need for the PMD to have the correct value prior to
> flow_ctrl_get() ? I didn't see one.


You are right, the value here does not take effect based on current 
implementation, I just correct it to align with datasheet.
I think it's not necessary to remove it here, it may be used in future.

The idea fix is: during init, the watermark is set with default value, so it is 
not necessary to read out from hw register during flow_ctl_get, 
But due to I40E_GLRPB_GHW limitation, it is shared by different ports on the 
same device, it is possible the value is changed on another port, but local 
variable not sync, so we have to read out register every flow_ctl_get.

Regards
Qi

> 
> >  /* Receive Average Packet Size in Byte*/  #define
> > I40E_PACKET_AVERAGE_SIZE 128
> >
> > @@ -3199,6 +3199,13 @@ i40e_flow_ctrl_get(struct rte_eth_dev *dev,
> struct rte_eth_fc_conf *fc_conf)
> > struct i40e_pf *pf =
> I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> >
> > fc_conf->pause_time = pf->fc_conf.pause_time;
> > +
> > +   /* read out from register, in case they are modified by other port */
> > +   pf->fc_conf.high_water[I40E_MAX_TRAFFIC_CLASS] =
> > +   I40E_READ_REG(hw, I40E_GLRPB_GHW) >> I40E_KILOSHIFT;
> > +   pf->fc_conf.low_water[I40E_MAX_TRAFFIC_CLASS] =
> > +   I40E_READ_REG(hw, I40E_GLRPB_GLW) >> I40E_KILOSHIFT;
> > +
> > fc_conf->high_water =
> pf->fc_conf.high_water[I40E_MAX_TRAFFIC_CLASS];
> > fc_conf->low_water =
> pf->fc_conf.low_water[I40E_MAX_TRAFFIC_CLASS];
> >
> >