* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Steffen Klassert @ 2014-11-12 9:12 UTC (permalink / raw)
To: Herbert Xu; +Cc: Ming Liu, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <20141112085148.GA26268@gondor.apana.org.au>
On Wed, Nov 12, 2014 at 04:51:48PM +0800, Herbert Xu wrote:
> On Wed, Nov 12, 2014 at 09:41:38AM +0100, Steffen Klassert wrote:
> >
> > Can't we just use cryptd unconditionally to fix this reordering problem?
>
> I think the idea is that most of the time cryptd isn't required
> so we want to stick with direct processing to lower latency.
Yes, I thought that. But is it really the case that doing it
asynchronous increases the latency? I tried this some time
ago and as far as I remember there was not too much difference
between the direct and the asynchronous case. Might depend
on the usecase of course.
>
> I think the simplest fix would be to punt to cryptd as long as
> there are cryptd requests queued.
This would be the second option. We may need to adjust the
maximum cryptd queue lenght then, as the networking receive
softirq might enqueue a lot of packets before cryptd can
process them.
^ permalink raw reply
* Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
From: Johan Hovold @ 2014-11-12 9:19 UTC (permalink / raw)
To: Sascha Hauer
Cc: Johan Hovold, Mark Rutland, Florian Fainelli, David S. Miller,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <20141112070127.GF30369@pengutronix.de>
On Wed, Nov 12, 2014 at 08:01:27AM +0100, Sascha Hauer wrote:
> On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> > On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > > documentation.
> > > >
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > ---
> > > > Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > > index a1bab5eaae02..9b08dd6551dd 100644
> > > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > > @@ -19,6 +19,11 @@ Optional properties:
> > > >
> > > > See the respective PHY datasheet for the mode values.
> > > >
> > > > + - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
> > > > +
> > > > + Whether 25 MHz (rather than 50 Mhz) clock mode is selected
> > > > + when the rmii_ref_clk_sel bit is set.
> > >
> > > s/_/-/ in property names please.
> >
> > Ouch, copied from variable name, sorry.
> >
> > > That said, I don't follow the meaning. Does this cause the kernel to do
> > > something different, or is is simply that a 25MHz ref clock is wired up?
> >
> > Yes, the driver currently sets this configuration bit based on a common
> > clock binding.
> >
> > However, it turns out the meaning of the bit is reversed on some PHY
> > variants. On most PHYs 50 MHz mode is selected by setting this bit,
> > whereas on the PHYs that need this new property, setting it selects 25
> > MHz mode instead.
>
> Maybe rename the property to something like rmii-ref-clk-25mhz-active-high
> then? Also you should probably make it more explicit that this is a
> hardware property and not for adjusting the clock.
You're right, but how about calling it
micrel,rmii-reference-clock-select-inverted
Then no one will set it believing it will change the clock mode and
without reading the binding doc first.
The description could then read something like
micrel,rmii-reference-clock-select-inverted: RMII Reference
Clock Select bit is inverted
The RMII Reference Clock Select bit is inverted so that setting
it selects 25 MHz rather than 50 MHz clock mode.
Note that this is only needed for PHY variants that has this bit
inverted and that a clock reference ("rmii-ref" below) is always
needed to select the actual mode.
> It's not very nice from Micrel to create Phys with exactly the same
> device id but the meaning of this bit reversed.
Not very nice at all.
Thanks,
Johan
^ permalink raw reply
* [PATCH net-next] cxgb4: Fix static checker warning
From: Hariprasad Shenai @ 2014-11-12 9:31 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, anish, nirranjan, kumaras, Hariprasad Shenai
Fix static checker warning that got introduced in commit e2ac9628959cc152
("cxgb4: Cleanup macros so they follow the same style and look consistent, part
2") due to accidental checkin of bogus line.
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index d13d36a..660bf0f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5752,7 +5752,6 @@ static int adap_init0(struct adapter *adap)
"No Configuration File present "
"on adapter. Using hard-wired "
"configuration parameters.\n");
- goto bye;
ret = adap_init0_no_config(adap, reset);
}
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 2/5] stmmac: pci: use managed resources
From: Andy Shevchenko @ 2014-11-12 9:28 UTC (permalink / raw)
To: Rayagond Kokatanur
Cc: Giuseppe CAVALLARO, Sergei Shtylyov, netdev, Kweh Hock Leong,
David S. Miller, Vince Bridgers
In-Reply-To: <CAJ3bTp79FLhOCvvQNG=+C17v5TOzy2ZvbyC24OQWTr2mmJ4OCw@mail.gmail.com>
On Wed, 2014-11-12 at 10:44 +0530, Rayagond Kokatanur wrote:
> Hi Andy,
>
> Yes, I tested this pci driver on XILINX FPGA setup and its true that
> we can man any bar number by changing the configuration in bit-map.
>
> But its always good to keep that for loop which try to map any bar
> number between 0 - 5, the new code breaks if bar number is other than
> zero.
>
> We can make driver generic by keeping that loop.
Thanks for clarification, I had redone this in v2 which is applied.
>
> wwr
> Rayagond
>
> On Thu, Oct 30, 2014 at 1:35 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, 2014-10-27 at 16:28 +0100, Giuseppe CAVALLARO wrote:
> >> On 10/22/2014 10:36 AM, Andy Shevchenko wrote:
> >> > So, I was trying to find any specification on public regarding to boards
> >> > that have this IP, no luck so far. I guess that that code was created
> >> > due to XILINX FPGA usage which probably can provide any BAR user wants
> >> > to. Thus, I imply that in real applications the BAR most probably will
> >> > be 0. However, I left variable which can be overridden in future
> >> > (regarding to PCI ID).
> >> >
> >> > It would be nice to hear someone from ST about this. Giuseppe?
> >>
> >> Hello Andy
> >>
> >> this chip is on ST SoCs since long time but embedded. I have no PCI
> >> card. Added Rayagond on copy too
> >
> > Rayagond, what do you think about changing an approach that is used to
> > get resources from PCI?
> >
> > --
> > Andy Shevchenko <andriy.shevchenko@intel.com>
> > Intel Finland Oy
> >
--
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH 8/9] net: phy: micrel: clean up led-mode setup
From: Johan Hovold @ 2014-11-12 9:29 UTC (permalink / raw)
To: Florian Fainelli
Cc: Sergei Shtylyov, Johan Hovold, David S. Miller, linux-kernel,
netdev
In-Reply-To: <546282EA.7040508@gmail.com>
On Tue, Nov 11, 2014 at 01:43:06PM -0800, Florian Fainelli wrote:
> On 11/11/2014 01:41 PM, Sergei Shtylyov wrote:
> > Hello.
> >
> > On 11/11/2014 10:00 PM, Johan Hovold wrote:
> >
> >> Clean up led-mode setup by introducing proper defines for PHY Control
> >> registers 1 and 2 and only passing the register to the setup function.
> >
> > Not sure that's really better that it was before (modulo naming).
>
> We do have proper error handling in kszphy_setup_led() which is already
> an improvement.
This also means handling the led mode in one place rather than spreading
it all over the driver, with multiple config functions providing one of
the same two combinations of register and shift.
It's usefulness will perhaps become more apparent if you look at the
follow up patches, which does further refactoring and store the led-mode
register in the type data.
Johan
^ permalink raw reply
* RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform
From: Stam, Michel [FINT] @ 2014-11-12 9:49 UTC (permalink / raw)
To: Ben Hutchings, Charles Keepax
Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <1415751802.3398.107.camel@decadent.org.uk>
Hello Ben,
Regarding the code snippet;
Good question, The original code didn't do this either, which is why I left it as it is. It could cause undesirable behaviour, agreed.
After a quick driver examination: I do see that asix_set_sw_mii and asix_set_hw_mii are called prior to the actual write (asix_mdio_write), it may be that this takes care of ensuring data is written to the chip, as asix_write_cmd waits for usbnet_write_cmd to send (and acknowledge) a USB CONTROL MSG. A lock to the phy is held during this time.
If I recall my USB knowledge, control messages are acknowledged, which would ensure data is written to the chip. Whether the ASIX requires further delay I would not know. I would have to dive deeper into the timings of the ASIX chip and the driver behaviour to see if that would be the case.
Kind regards,
Michel Stam
-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk]
Sent: Wednesday, November 12, 2014 1:23 AM
To: Charles Keepax
Cc: Stam, Michel [FINT]; Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform
On Tue, 2014-11-04 at 20:09 +0000, Charles Keepax wrote:
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> >
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere
> > >(in
> > this case on arndale). This is now a regression of functionality
> > from 3.17.
> > >
> > >I think it would better to revert the change now and with less
> > >hurry
> > introduce a ethtool fix that doesn't break arndale.
> >
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself.
> > Fixing the armdale will then cause breakage in other
> > implementations, such as ours. Blankly reverting breaks other peoples' implementations.
> >
> > The PHY reset is the thing that breaks ethtool support, so any fix
> > that appeases all would have to take existing PHY state into account.
[...]
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> + int reg;
> u16 rx_ctl;
>
> ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
> msleep(150);
>
> asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
> - asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> - ADVERTISE_ALL | ADVERTISE_CSMA);
> + reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
> + if (!reg)
> + reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> + asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> + reg);
[...]
Why is there no sleep after setting the RESET bit? Doesn't that make the following register writes unreliable?
Ben.
--
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
- Carolyn Scheppner
^ permalink raw reply
* Re: [PATCH v1 net-next 1/2] bonding: Expand speed type bits of the AD Port Key
From: Jianhua Xie @ 2014-11-12 9:53 UTC (permalink / raw)
To: Jay Vosburgh, David Miller; +Cc: netdev, vfalico, andy
In-Reply-To: <19882.1415735238@famine>
Thanks you two for the valuable comments.
If my understanding is right, it is encouraged to use a counter
rather than a bitmask for the speed field, right?
if yes, how many bits are better to use for current speed and
future speed (like 100Gbps/400Gbps and etc.)? I am not sure
that 5 bits are enough (2**5=32) or not. And I am clear to keep
"the duplex bit in the key " in my mind.
if not, what's your recommendation please?
Thanks & Best Regards,
Jianhua
在 2014年11月12日 03:47, Jay Vosburgh 写道:
> David Miller <davem@davemloft.net> wrote:
>
>> From: Xie Jianhua <Jianhua.Xie@freescale.com>
>> Date: Mon, 10 Nov 2014 15:16:40 +0800
>>
>>> From: Jianhua Xie <Jianhua.Xie@freescale.com>
>>>
>>> Port Key was determined as 16 bits according to the link speed,
>>> duplex and user key (which is yet not supported), in which key
>>> speed was 5 bits for 1Mbps/10Mbps/100Mbps/1Gbps/10Gbps as below:
>>> --------------------------------------------------------------
>>> Port key :| User key | Speed | Duplex|
>>> --------------------------------------------------------------
>>> 16 6 1 0
>>> This patch is expanding speed type from 5 bits to 9 bits for other
>>> speed 2.5Gbps/20Gbps/40Gbps/56Gbps and shrinking user key from 10
>>> bits to 6 bits. New Port Key looks like below:
>>> --------------------------------------------------------------
>>> Port key :| User key | Speed | Duplex|
>>> --------------------------------------------------------------
>>> 16 10 1 0
>>>
>> Do we determine the layout of this value all ourselves?
> Yes, we do. The precise format of the port key is not defined
> by the standard; IEEE 802.1AX 5.3.5, "Capability identification":
>
> "A given Key value is meaningful only in the context of the System that
> allocates it; there is no global significance to Key values."
>
> and
>
> "When a System assigns an operational Key value to a set of ports, it
> signifies that, in the absence of other constraints, the current
> operational state of the set of ports allows any subset of that set of
> ports (including the entire set) to be aggregated together from the
> perspective of the System making the assignment."
>
> So, basically, it's a magic cookie that indicates that all ports
> on a particular system with the same key value are suitable to be
> aggregated together.
>
>> If not, then is it exported to anything user-visible that we
>> might be breaking?
> The key values are not user-visible, and the "user" settable
> portion of the key has never been implemented.
>
>> If it is private, it makes no sense to use a bitmask for the speed.
>> We should instead change the field to be some numerically increasing
>> value.
>>
>> Otherwise we'll run out of bits again and keep having to adjust the
>> field layout more often than we really need to.
> Agreed.
>
> Also note that there are some internal dependencies within
> bonding on the format; in particular the duplex bit in the key is used
> to determine if a port is LACP-capable, and that functionality needs to
> be preserved.
>
> -J
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* [PATCH net] net: ptp: fix time stamp matching logic for VLAN packets.
From: Richard Cochran @ 2014-11-12 10:33 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Stefan Sørensen
Commit ae5c6c6d "ptp: Classify ptp over ip over vlan packets" changed the
code in two drivers that matches time stamps with PTP frames, with the goal
of allowing VLAN tagged PTP packets to receive hardware time stamps.
However, that commit failed to account for the VLAN header when parsing
IPv4 packets. This patch fixes those two drivers to correctly match VLAN
tagged IPv4/UDP PTP messages with their time stamps.
This patch should also be applied to v3.17.
Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/ti/cpts.c | 2 +-
drivers/net/phy/dp83640.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index ab92f67..4a4388b 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -264,7 +264,7 @@ static int cpts_match(struct sk_buff *skb, unsigned int ptp_class,
switch (ptp_class & PTP_CLASS_PMASK) {
case PTP_CLASS_IPV4:
- offset += ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
+ offset += ETH_HLEN + IPV4_HLEN(data + offset) + UDP_HLEN;
break;
case PTP_CLASS_IPV6:
offset += ETH_HLEN + IP6_HLEN + UDP_HLEN;
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 2954052..e22e602 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -791,7 +791,7 @@ static int match(struct sk_buff *skb, unsigned int type, struct rxts *rxts)
switch (type & PTP_CLASS_PMASK) {
case PTP_CLASS_IPV4:
- offset += ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
+ offset += ETH_HLEN + IPV4_HLEN(data + offset) + UDP_HLEN;
break;
case PTP_CLASS_IPV6:
offset += ETH_HLEN + IP6_HLEN + UDP_HLEN;
@@ -934,7 +934,7 @@ static int is_sync(struct sk_buff *skb, int type)
switch (type & PTP_CLASS_PMASK) {
case PTP_CLASS_IPV4:
- offset += ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
+ offset += ETH_HLEN + IPV4_HLEN(data + offset) + UDP_HLEN;
break;
case PTP_CLASS_IPV6:
offset += ETH_HLEN + IP6_HLEN + UDP_HLEN;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Ming Liu @ 2014-11-12 10:41 UTC (permalink / raw)
To: Herbert Xu; +Cc: Steffen Klassert, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <20141112085148.GA26268@gondor.apana.org.au>
On 11/12/2014 04:51 PM, Herbert Xu wrote:
> On Wed, Nov 12, 2014 at 09:41:38AM +0100, Steffen Klassert wrote:
>> Can't we just use cryptd unconditionally to fix this reordering problem?
> I think the idea is that most of the time cryptd isn't required
> so we want to stick with direct processing to lower latency.
>
> I think the simplest fix would be to punt to cryptd as long as
> there are cryptd requests queued.
I've tried that method when I started to think about the fix, but it
will cause 2 other issues per test while resolving the reordering one,
as follows:
1 The work queue can not handle so many packets when the traffic is very
high(over 200M/S), and it would drop most of them when the queue length
is beyond CRYPTD_MAX_CPU_QLEN.
2 Soft lockups are observed, per my analysis, the cause is: in some
extreme cases, all packets would be sent to the cryptd once there are
requests already in it, this will let the net poll functions return
pretty fast, and lead too many hw interrupts triggered in a certain period.
//Ming Liu
>
> Cheers,
^ permalink raw reply
* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Ming Liu @ 2014-11-12 10:41 UTC (permalink / raw)
To: Steffen Klassert; +Cc: herbert, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <20141112084138.GL6390@secunet.com>
On 11/12/2014 04:41 PM, Steffen Klassert wrote:
> On Wed, Nov 12, 2014 at 01:49:31PM +0800, Ming Liu wrote:
>> So far, the encryption/decryption are asynchronously processed in
>> softirq and cryptd which would result in a implicit order of data,
>> therefore leads IPSec stack also out of order while encapsulating
>> or decapsulating packets.
>>
>> Consider the following scenario:
>>
>> DECRYPTION INBOUND
>> |
>> |
>> +-----Packet A
>> | |
>> | |
>> | Packet B
>> | |
>> (cryptd) | | (software interrupts)
>> | ......
>> | |
>> | |
>> | Packet B(decrypted)
>> | |
>> | |
>> +---> Packet A(decrypted)
>> |
>> |
>> DECRYPTION OUTBOUND
>>
>> Add cryptd_flush_queue function fixing it by being called from softirq
>> to flush all previous queued elements before processing a new one. To
>> prevent cryptd_flush_queue() being accessed from software interrupts,
>> local_bh_disable/enable needs to be relocated in several places.
>>
>> Signed-off-by: Ming Liu <ming.liu@windriver.com>
>> ---
>> I was told that I need resend this patch with git, so here it is:
>>
>> I've figured out a new patch for this issue reported by me previously,
>> the basic idea is adding a cryptd_flush_queue function fixing it by
>> being called from softirq to flush all previous queued elements
>> before processing a new one, and it works very well so far per my test.
> I doubt that this approach can work. The cryptd encrypt/decrypt functions
> assume to be called from a workqueue worker, they can't be called from
> atomic context.
Yes, you are correct, I am on board, I need rework the patch.
>
> Can't we just use cryptd unconditionally to fix this reordering problem?
>> crypto/ablk_helper.c | 10 ++++-
>> crypto/cryptd.c | 107 ++++++++++++++++++++++++++++++++++++++++--------
>> include/crypto/cryptd.h | 13 ++++++
>> 3 files changed, 111 insertions(+), 19 deletions(-)
>>
>> diff --git a/crypto/ablk_helper.c b/crypto/ablk_helper.c
>> index ffe7278..600a70f 100644
>> --- a/crypto/ablk_helper.c
>> +++ b/crypto/ablk_helper.c
>> @@ -70,16 +70,19 @@ int ablk_encrypt(struct ablkcipher_request *req)
>> {
>> struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
>> struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
>> + struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
>> + crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
>>
>> if (!may_use_simd()) {
>> struct ablkcipher_request *cryptd_req =
>> ablkcipher_request_ctx(req);
>>
>> *cryptd_req = *req;
>> - ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
>> + cryptd_req->base.tfm = req_tfm;
>>
>> return crypto_ablkcipher_encrypt(cryptd_req);
>> } else {
>> + cryptd_flush_queue(req_tfm, CRYPTD_ENCRYPT);
> Where is CRYPTD_ENCRYPT defined?
> This does not even compile when applied to the cryptodev tree.
Sorry, it really should be CRYPTD_BLKCIPHER_ENCRYPT, I made the original
patch against 3.4 kernel, there must be something wrong when I adjusted
it to mainline kernel, sorry for that.
>
>> return __ablk_encrypt(req);
>> }
>> }
>> @@ -89,13 +92,15 @@ int ablk_decrypt(struct ablkcipher_request *req)
>> {
>> struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req);
>> struct async_helper_ctx *ctx = crypto_ablkcipher_ctx(tfm);
>> + struct crypto_tfm *req_tfm = crypto_ablkcipher_tfm(
>> + crypto_ablkcipher_crt(&ctx->cryptd_tfm->base)->base);
>>
>> if (!may_use_simd()) {
>> struct ablkcipher_request *cryptd_req =
>> ablkcipher_request_ctx(req);
>>
>> *cryptd_req = *req;
>> - ablkcipher_request_set_tfm(cryptd_req, &ctx->cryptd_tfm->base);
>> + cryptd_req->base.tfm = req_tfm;
>>
>> return crypto_ablkcipher_decrypt(cryptd_req);
>> } else {
>> @@ -105,6 +110,7 @@ int ablk_decrypt(struct ablkcipher_request *req)
>> desc.info = req->info;
>> desc.flags = 0;
>>
>> + cryptd_flush_queue(req_tfm, CRYPTD_DECRYPT);
> Same here.
Should be CRYPTD_BLKCIPHER_DECRYPT.
>
>> return crypto_blkcipher_crt(desc.tfm)->decrypt(
>> &desc, req->dst, req->src, req->nbytes);
>> }
>> diff --git a/crypto/cryptd.c b/crypto/cryptd.c
>> index e592c90..0b387a1 100644
>> --- a/crypto/cryptd.c
>> +++ b/crypto/cryptd.c
>> @@ -119,11 +119,13 @@ static int cryptd_enqueue_request(struct cryptd_queue *queue,
>> int cpu, err;
>> struct cryptd_cpu_queue *cpu_queue;
>>
>> + local_bh_disable();
>> cpu = get_cpu();
>> cpu_queue = this_cpu_ptr(queue->cpu_queue);
>> err = crypto_enqueue_request(&cpu_queue->queue, request);
>> queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
>> put_cpu();
>> + local_bh_enable();
>>
>> return err;
>> }
>> @@ -147,11 +149,9 @@ static void cryptd_queue_worker(struct work_struct *work)
>> preempt_disable();
>> backlog = crypto_get_backlog(&cpu_queue->queue);
>> req = crypto_dequeue_request(&cpu_queue->queue);
>> - preempt_enable();
>> - local_bh_enable();
> Everything below the local_bh_enable() should not run in atomic context
> as the subsequent functions may set the CRYPTO_TFM_REQ_MAY_SLEEP flag.
If I turn off all the CRYPTO_TFM_REQ_MAY_SLEEP in cryptd.c, is that
going to work?
>
>>
>> if (!req)
>> - return;
>> + goto out;
>>
>> if (backlog)
>> backlog->complete(backlog, -EINPROGRESS);
>> @@ -159,6 +159,9 @@ static void cryptd_queue_worker(struct work_struct *work)
>>
>> if (cpu_queue->queue.qlen)
>> queue_work(kcrypto_wq, &cpu_queue->work);
>> +out:
>> + preempt_enable();
>> + local_bh_enable();
>> }
> ...
>
>>
>> +void cryptd_flush_queue(struct crypto_tfm *tfm, cryptd_type_t type)
>> +{
>> + struct crypto_instance *inst = crypto_tfm_alg_instance(tfm);
>> + struct cryptd_instance_ctx *ictx = crypto_instance_ctx(inst);
>> + struct cryptd_queue *cryptd_queue = ictx->queue;
>> + struct cryptd_cpu_queue *cpu_queue;
>> + struct crypto_queue *queue;
>> + struct crypto_async_request *req, *tmp, *backlog;
>> + crypto_completion_t complete;
>> + int cpu;
>> + unsigned int len;
>> +
>> + switch (type) {
>> + case CRYPTD_BLKCIPHER_ENCRYPT:
>> + complete = cryptd_blkcipher_encrypt;
>> + break;
>> + case CRYPTD_BLKCIPHER_DECRYPT:
>> + complete = cryptd_blkcipher_decrypt;
>> + break;
>> + case CRYPTD_HASH_INIT:
>> + complete = cryptd_hash_init;
>> + break;
>> + case CRYPTD_HASH_UPDATE:
>> + complete = cryptd_hash_update;
>> + break;
>> + case CRYPTD_HASH_FINAL:
>> + complete = cryptd_hash_final;
>> + break;
>> + case CRYPTD_HASH_FINUP:
>> + complete = cryptd_hash_finup;
>> + break;
>> + case CRYPTD_HASH_DIGEST:
>> + complete = cryptd_hash_digest;
>> + break;
>> + case CRYPTD_AEAD_ENCRYPT:
>> + complete = cryptd_aead_encrypt;
>> + break;
>> + case CRYPTD_AEAD_DECRYPT:
>> + complete = cryptd_aead_decrypt;
>> + break;
>> + default:
>> + complete = NULL;
>> + }
>> +
>> + if (complete == NULL)
>> + return;
>> +
>> + local_bh_disable();
>> + cpu = get_cpu();
>> + cpu_queue = this_cpu_ptr(cryptd_queue->cpu_queue);
>> + queue = &cpu_queue->queue;
>> + len = queue->qlen;
>> +
>> + if (!len)
>> + goto out;
>> +
>> + list_for_each_entry_safe(req, tmp, &queue->list, list) {
>> + if (req->complete == complete) {
>> + queue->qlen--;
>> +
>> + if (queue->backlog != &queue->list) {
>> + backlog = container_of(queue->backlog,
>> + struct crypto_async_request, list);
>> + queue->backlog = queue->backlog->next;
>> + } else
>> + backlog = NULL;
>> +
>> + list_del(&req->list);
>> +
>> + if (backlog)
>> + backlog->complete(backlog, -EINPROGRESS);
>> + req->complete(req, 0);
> Same here, the complete function can not run in atomic context.
> Also, this can not ensure that the request is finalized.
> Subsequent algorithms may run asynchronously too, so this
> does not fix the reordering problem completely.
I do not know that before. Then I need rework the patch.
//Ming Liu
>
>
>
^ permalink raw reply
* Re: [PATCH nf] netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse
From: Jörg Marx @ 2014-11-12 10:57 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Pablo Neira Ayuso
Cc: programme110, netfilter-devel, Florian Westphal, netdev,
Patrick McHardy
In-Reply-To: <20141112083500.5404e5f4@redhat.com>
On 12.11.2014 08:35, Jesper Dangaard Brouer wrote:
Hi,
I wrote the patch in 2010, so find some arguments below:
>>> > > + nf_ct_del_from_dying_or_unconfirmed_list(ct);
>>> > >
>>> > > if (unlikely(nf_ct_is_dying(ct))) {
>>> > > + nf_ct_add_to_dying_list(ct);
>>> > > nf_conntrack_double_unlock(hash, reply_hash);
>>> > > local_bh_enable();
>>> > > return NF_ACCEPT;
>> >
>> > Not directly related to your patch, but I don't find a good reason why
>> > we're accepting this packet.
>> >
>> > If the conntrack from the unconfirmed list is dying, then the object
>> > will be released by when the packet leaves the stack to its
>> > destination. With stateful filtering depending in place, the follow up
>> > packet in the reply direction will likely be considered invalid (if
>> > tcp tracking is on). Fortunately for us, the origin will likely
>> > retransmit the syn again, so the ct will be setup accordingly.
>> >
>> > So, why should we allow this to go through?
> True, it also seems strange to me that we accept this packet.
The raise was triggered in a scenario when we tested high-load IPsec
tunnels and flushed the conntrack hashs from userspace.
For me there is little difference in choosing DROP or ACCEPT as verdict.
The packet/skb belongs to a formerly allowed connection, most likely
this connection is still allowed (but the conntrack hash entry is about
to be removed due to userspace is flushing the conntrack table).
To minimize the impact (lost packets -> retransmit) I decided to allow
the skb in flight, so were is no lost packet at this place.
When the connection is not allowed anymore (but was allowed up to now,
because the hash entry exists), the impact is one last packet 'slipping
through'.
Today I would still decide the way I did in 2010.
>
>> > This return verdict was introduced in: fc35077 ("netfilter:
>> > nf_conntrack: fix a race in __nf_conntrack_confirm against
>> > nf_ct_get_next_corpse()") btw.
> And the commit does not argue why NF_ACCEPT was chosen...
>
> -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel
> Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn:
> http://www.linkedin.com/in/brouer
best regards
Jörg Marx
--
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] net: pxa168_eth: move SET_NETDEV_DEV a bit earlier
From: Jisheng Zhang @ 2014-11-12 11:08 UTC (permalink / raw)
To: davem, antoine.tenart, arnd, sebastian.hesselbarth
Cc: netdev, linux-kernel, linux-arm-kernel, Jisheng Zhang
This is to ensure the net_device's dev.parent is set before we used it
in dma_zalloc_coherent() from init_hash_table().
Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
drivers/net/ethernet/marvell/pxa168_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 21ddece..38f7cee 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -1540,8 +1540,8 @@ static int pxa168_eth_probe(struct platform_device *pdev)
if (err)
goto err_free_mdio;
- pxa168_init_hw(pep);
SET_NETDEV_DEV(dev, &pdev->dev);
+ pxa168_init_hw(pep);
err = register_netdev(dev);
if (err)
goto err_mdiobus;
--
2.1.3
^ permalink raw reply related
* Re: [PATCH v1 net-next 1/2] bonding: Expand speed type bits of the AD Port Key
From: Veaceslav Falico @ 2014-11-12 11:20 UTC (permalink / raw)
To: Jianhua Xie; +Cc: Jay Vosburgh, David Miller, netdev, andy
In-Reply-To: <54632E25.3000205@freescale.com>
On Wed, Nov 12, 2014 at 05:53:41PM +0800, Jianhua Xie wrote:
>Thanks you two for the valuable comments.
>
>If my understanding is right, it is encouraged to use a counter
>rather than a bitmask for the speed field, right?
>
>if yes, how many bits are better to use for current speed and
>future speed (like 100Gbps/400Gbps and etc.)? I am not sure
>that 5 bits are enough (2**5=32) or not. And I am clear to keep
>"the duplex bit in the key " in my mind.
>
>if not, what's your recommendation please?
As it's visible to bonding only, I guess a simple enum should do the trick.
No need to invent something special, and it'll fit nicely with other enums
from AD.
>
>Thanks & Best Regards,
>Jianhua
>
>在 2014年11月12日 03:47, Jay Vosburgh 写道:
>>David Miller <davem@davemloft.net> wrote:
>>
>>>From: Xie Jianhua <Jianhua.Xie@freescale.com>
>>>Date: Mon, 10 Nov 2014 15:16:40 +0800
>>>
>>>>From: Jianhua Xie <Jianhua.Xie@freescale.com>
>>>>
>>>>Port Key was determined as 16 bits according to the link speed,
>>>>duplex and user key (which is yet not supported), in which key
>>>>speed was 5 bits for 1Mbps/10Mbps/100Mbps/1Gbps/10Gbps as below:
>>>>--------------------------------------------------------------
>>>>Port key :| User key | Speed | Duplex|
>>>>--------------------------------------------------------------
>>>>16 6 1 0
>>>>This patch is expanding speed type from 5 bits to 9 bits for other
>>>>speed 2.5Gbps/20Gbps/40Gbps/56Gbps and shrinking user key from 10
>>>>bits to 6 bits. New Port Key looks like below:
>>>>--------------------------------------------------------------
>>>>Port key :| User key | Speed | Duplex|
>>>>--------------------------------------------------------------
>>>>16 10 1 0
>>>>
>>>Do we determine the layout of this value all ourselves?
>> Yes, we do. The precise format of the port key is not defined
>>by the standard; IEEE 802.1AX 5.3.5, "Capability identification":
>>
>>"A given Key value is meaningful only in the context of the System that
>>allocates it; there is no global significance to Key values."
>>
>> and
>>
>>"When a System assigns an operational Key value to a set of ports, it
>>signifies that, in the absence of other constraints, the current
>>operational state of the set of ports allows any subset of that set of
>>ports (including the entire set) to be aggregated together from the
>>perspective of the System making the assignment."
>>
>> So, basically, it's a magic cookie that indicates that all ports
>>on a particular system with the same key value are suitable to be
>>aggregated together.
>>
>>>If not, then is it exported to anything user-visible that we
>>>might be breaking?
>> The key values are not user-visible, and the "user" settable
>>portion of the key has never been implemented.
>>
>>>If it is private, it makes no sense to use a bitmask for the speed.
>>>We should instead change the field to be some numerically increasing
>>>value.
>>>
>>>Otherwise we'll run out of bits again and keep having to adjust the
>>>field layout more often than we really need to.
>> Agreed.
>>
>> Also note that there are some internal dependencies within
>>bonding on the format; in particular the duplex bit in the key is used
>>to determine if a port is LACP-capable, and that functionality needs to
>>be preserved.
>>
>> -J
>>
>>---
>> -Jay Vosburgh, jay.vosburgh@canonical.com
>
^ permalink raw reply
* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Steffen Klassert @ 2014-11-12 11:43 UTC (permalink / raw)
To: Ming Liu; +Cc: Herbert Xu, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <5463395A.5040200@windriver.com>
On Wed, Nov 12, 2014 at 06:41:30PM +0800, Ming Liu wrote:
> On 11/12/2014 04:51 PM, Herbert Xu wrote:
> >On Wed, Nov 12, 2014 at 09:41:38AM +0100, Steffen Klassert wrote:
> >>Can't we just use cryptd unconditionally to fix this reordering problem?
> >I think the idea is that most of the time cryptd isn't required
> >so we want to stick with direct processing to lower latency.
> >
> >I think the simplest fix would be to punt to cryptd as long as
> >there are cryptd requests queued.
> I've tried that method when I started to think about the fix, but it
> will cause 2 other issues per test while resolving the reordering
> one, as follows:
> 1 The work queue can not handle so many packets when the traffic is
> very high(over 200M/S), and it would drop most of them when the
> queue length is beyond CRYPTD_MAX_CPU_QLEN.
That's why I've proposed to adjust CRYPTD_MAX_CPU_QLEN in my other mail.
But anyway, it still does not fix the reorder problem completely.
We still have a problem if subsequent algorithms run asynchronously
or if we get interrupted while we are processing the last request
from the queue.
I think we have only two options, either processing all calls
directly or use cryptd unconditionally. Mixing direct and
asynchronous calls will lead to problems.
If we don't want to use cryptd unconditionally, we could use
direct calls for all requests. If the fpu is not usable, we
maybe could fallback to an algorithm that does not need the
fpu, such as aes-generic.
^ permalink raw reply
* Re: [PATCH] crypto: aesni-intel - avoid IPsec re-ordering
From: Steffen Klassert @ 2014-11-12 11:48 UTC (permalink / raw)
To: Ming Liu; +Cc: herbert, davem, ying.xue, linux-crypto, netdev
In-Reply-To: <54633958.2080705@windriver.com>
On Wed, Nov 12, 2014 at 06:41:28PM +0800, Ming Liu wrote:
> On 11/12/2014 04:41 PM, Steffen Klassert wrote:
> >On Wed, Nov 12, 2014 at 01:49:31PM +0800, Ming Liu wrote:
> >> }
> >>@@ -147,11 +149,9 @@ static void cryptd_queue_worker(struct work_struct *work)
> >> preempt_disable();
> >> backlog = crypto_get_backlog(&cpu_queue->queue);
> >> req = crypto_dequeue_request(&cpu_queue->queue);
> >>- preempt_enable();
> >>- local_bh_enable();
> >Everything below the local_bh_enable() should not run in atomic context
> >as the subsequent functions may set the CRYPTO_TFM_REQ_MAY_SLEEP flag.
> If I turn off all the CRYPTO_TFM_REQ_MAY_SLEEP in cryptd.c, is that
> going to work?
Well, this might make the cryptd function accessible in atomic context,
but it does not solve the other problems with this approach. Also,
cryptd can be used to move requests out of atomic context and I think
it should stay as it is.
^ permalink raw reply
* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Aleksander Morgado @ 2014-11-12 11:54 UTC (permalink / raw)
To: Dan Williams
Cc: sedat.dilek, Greg KH, David S. Miller, netdev@vger.kernel.org,
linux-usb
In-Reply-To: <1415120132.31049.11.camel@dcbw.local>
>
> NetworkManager uses ModemManager for all WWAN control, NM only handles
> the configuration storage and IP addressing parts of the setup.
> ModemManager handles modem hardware detection, capability detection,
> WWAN registration and setup, signal strength reporting, network
> connection initiation, and bearer IP addressing method determination.
>
> If you want more information from ModemManager, you can run "mmcli
> --help" or "mmcli --set-logging=debug".
>
That (mmcli) will only be available in ModemManager >= 1.x, BTW. And
beware of --set-logging=debug, as syslog may limit those if MM sends
too many too fast. In general I usually prefer to ask for --debug
output just in case:
http://www.freedesktop.org/wiki/Software/ModemManager/Debugging/
I saw the reporter talk about Ubuntu Precise (12.04) and Trusty
(14.04), so only in the latter mmcli will be available (if I'm not
mistaken). For Precise, this Ubuntu page shows how to enable debug
logs:
https://wiki.ubuntu.com/DebuggingModemmanager
--
Aleksander
https://aleksander.es
^ permalink raw reply
* Re: [patch net-next 1/2] net: move vlan pop/push functions into common code
From: Jiri Pirko @ 2014-11-12 11:59 UTC (permalink / raw)
To: Pravin Shelar
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
willemb, Daniel Borkmann, mst, fw, Paul.Durrant, Thomas Graf
In-Reply-To: <CALnjE+qRASQx8pcaB7z9D3_x2ybA4h-Uh2Ppn9EJVG3YU8gSGg@mail.gmail.com>
Tue, Nov 11, 2014 at 06:24:15PM CET, pshelar@nicira.com wrote:
>On Tue, Nov 11, 2014 at 2:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> So it can be used from out of openvswitch code.
>> Did couple of cosmetic changes on the way, namely variable naming and
>> adding support for 8021AD proto.
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>> include/linux/skbuff.h | 2 ++
>> net/core/skbuff.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++
>> net/openvswitch/actions.c | 76 ++---------------------------------------
>> 3 files changed, 90 insertions(+), 74 deletions(-)
>>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 103fbe8..3b0445c 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -2673,6 +2673,8 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet);
>> unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
>> struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
>> struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
>> +int skb_vlan_pop(struct sk_buff *skb);
>> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
>>
>> struct skb_checksum_ops {
>> __wsum (*update)(const void *mem, int len, __wsum wsum);
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index 7001896..75e53d4 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -4151,6 +4151,92 @@ err_free:
>> }
>> EXPORT_SYMBOL(skb_vlan_untag);
>>
>> +/* remove VLAN header from packet and update csum accordingly. */
>> +static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
>> +{
>> + struct vlan_hdr *vhdr;
>> + int err;
>> +
>> + if (!pskb_may_pull(skb, VLAN_ETH_HLEN))
>> + return -ENOMEM;
>> +
>> + if (!skb_cloned(skb) || skb_clone_writable(skb, VLAN_ETH_HLEN))
>> + return 0;
>> +
>> + err = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
>> + if (unlikely(err))
>> + return err;
>> +
>> + if (skb->ip_summed == CHECKSUM_COMPLETE)
>> + skb->csum = csum_sub(skb->csum, csum_partial(skb->data
>> + + (2 * ETH_ALEN), VLAN_HLEN, 0));
>> +
>> + vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
>> + *vlan_tci = ntohs(vhdr->h_vlan_TCI);
>> +
>> + memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
>> + __skb_pull(skb, VLAN_HLEN);
>> +
>> + vlan_set_encap_proto(skb, vhdr);
>> + skb->mac_header += VLAN_HLEN;
>> + if (skb_network_offset(skb) < ETH_HLEN)
>> + skb_set_network_header(skb, ETH_HLEN);
>> + skb_reset_mac_len(skb);
>> +
>> + return 0;
>> +}
>> +
>> +int skb_vlan_pop(struct sk_buff *skb)
>> +{
>> + u16 vlan_tci;
>> + __be16 vlan_proto;
>> + int err;
>> +
>> + if (likely(vlan_tx_tag_present(skb))) {
>> + skb->vlan_tci = 0;
>> + } else {
>> + if (unlikely((skb->protocol != htons(ETH_P_8021Q) &&
>> + skb->protocol != htons(ETH_P_8021AD)) ||
>> + skb->len < VLAN_ETH_HLEN))
>> + return 0;
>> +
>> + err = __skb_vlan_pop(skb, &vlan_tci);
>> + if (err)
>> + return err;
>> + }
>> + /* move next vlan tag to hw accel tag */
>> + if (likely((skb->protocol != htons(ETH_P_8021Q) &&
>> + skb->protocol != htons(ETH_P_8021AD)) ||
>> + skb->len < VLAN_ETH_HLEN))
>> + return 0;
>> +
>> + vlan_proto = skb->protocol;
>> + err = __skb_vlan_pop(skb, &vlan_tci);
>> + if (unlikely(err))
>> + return err;
>> +
>> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(skb_vlan_pop);
>> +
>> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
>> +{
>> + if (vlan_tx_tag_present(skb)) {
>> + /* push down current VLAN tag */
>> + if (!__vlan_put_tag(skb, skb->vlan_proto,
>> + vlan_tx_tag_get(skb)))
>> + return -ENOMEM;
>> +
>Since you are restructuring this code, can you also change
>__vlan_put_tag() to not free skb on error. So that these two new
>functions can have common error handling code.
That is not directly related to this patchset. I believe that should be
changed in separate patch later on.
^ permalink raw reply
* RE: phy/micrel: KSZ8031RNL RMII clock reconfiguration bug
From: Bruno Thomsen @ 2014-11-12 12:17 UTC (permalink / raw)
To: Johan Hovold
Cc: netdev@vger.kernel.org, f.fainelli@gmail.com,
s.hauer@pengutronix.de, bruno.thomsen@gmail.com,
linux-kernel@vger.kernel.org
In-Reply-To: <20141111194101.GI29789@localhost>
Hi Johan,
> As you may have seen by now, I've been working on refactoring the micrel phy driver to be able to use common initialisation code.
>
> Specifically, I've added generic support for disabling the broadcast address, which is what the MII_KSZPHY_OMSO write above does.
>
> Generally you want this to be the first thing you do in order to avoid unnecessary reconfigurations. If we ever were to allow concurrent probing this would also be a requirement.
>
> Could you provide some detail about the setup were you find that the PHY becomes unresponsive without your patch? Do you have more than one PHY on the bus? Using what addresses? And using what clock modes (i.e. 25 MHz or 50 MHz)?
>
> Also, what exactly do you mean by "unresponsive"? Are you still able to read the PHY registers for example?
I think it sounds like a good idea to refactor the init code.
My setup:
iMX28 processor with dual Ethernet MAC; FEC0 (enabled) and FEC1 (disabled).
There is a single KSZ8031 PHY that receives 50MHz RMII clock from the MAC.
I am unable to read PHY registers from both user-land tools and extra debug PHY reads in driver code.
Boot trace:
[ 22.277785] fec 800f0000.ethernet eth0: Freescale FEC PHY driver [Micrel KSZ8031] (mii_bus:phy_addr=800f0000.etherne:00, irq=-1)
[ 22.292527] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[ 24.276217] libphy: 800f0000.etherne:00 - Link is Up - 100/Full
[ 24.285094] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Venlig hilsen / Best regards
Kamstrup A/S <http://www.kamstrup.dk>
Bruno Thomsen
Development engineer
Technology
Kamstrup A/S
Industrivej 28
DK-8660 Skanderborg
Tel: +45 89 93 10 00
Fax: +45 89 93 10 01
Dir: +45 89 93 13 94
E-mail: bth@kamstrup.dk
Web: www.kamstrup.dk
^ permalink raw reply
* Re: [patch net-next 2/2] sched: introduce vlan action
From: Jamal Hadi Salim @ 2014-11-12 12:27 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang
Cc: netdev, David Miller, Pravin B Shelar, Tom Herbert, Eric Dumazet,
willemb, Daniel Borkmann, mst, Florian Westphal, Paul.Durrant,
Thomas Graf
In-Reply-To: <20141112074755.GA1882@nanopsycho.orion>
On 11/12/14 02:47, Jiri Pirko wrote:
> Wed, Nov 12, 2014 at 12:18:47AM CET, cwang@twopensource.com wrote:
>> I know vlan tag is not exactly the skb metadata, but still seems
>> fits in skbedit for me.
>
> I was thinking about that as well. It seems much clearer to add a new
> action.
>
Cong,
I think it is better to have all these "tunneling" activities
as a separate action each. It is cleaner from a usability perspective.
[e.g. it is not hard to express nat action with pedit action or take
checksum out of nat since we have a csum action), but makes sense to
have it separate)].
cheers,
jamal
^ permalink raw reply
* Re: [PATCH -next 2/2] seq_putc: Convert to return void and convert uses too.
From: Pablo Neira Ayuso @ 2014-11-12 12:29 UTC (permalink / raw)
To: Petr Mladek
Cc: Joe Perches, Steven Rostedt, Corey Minyard, Alexander Viro,
Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
openipmi-developer, linux-kernel, linux-fsdevel, netfilter-devel,
coreteam, netdev
In-Reply-To: <20141111134740.GB2567@pathway.suse.cz>
On Tue, Nov 11, 2014 at 02:47:40PM +0100, Petr Mladek wrote:
> On Mon 2014-11-10 10:58:57, Joe Perches wrote:
> > Using the return value of seq_putc is error-prone, so
> > make it return void instead.
> >
> > Reverse the logic in seq_putc to make it like seq_puts.
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
>
> Reviewed-by: Petr Mladek <pmladek@suse.cz>
>
> The changes are correct. The show() functions should return 0
> even when there is an overflow. They are called by traverse()
> from seq_read() that might increase the buffer size and try again.
Just in case you need this for the netfilter chunks:
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thanks.
^ permalink raw reply
* Re: [patch net-next 2/2] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-12 12:34 UTC (permalink / raw)
To: Cong Wang
Cc: netdev, David Miller, Jamal Hadi Salim, Pravin B Shelar,
Tom Herbert, Eric Dumazet, willemb, Daniel Borkmann, mst,
Florian Westphal, Paul.Durrant, Thomas Graf
In-Reply-To: <CAHA+R7O=_h4HEF01wFacteawVX-2uvMiPjUAExKV6GphSJySzg@mail.gmail.com>
Tue, Nov 11, 2014 at 11:33:21PM CET, cwang@twopensource.com wrote:
>On Tue, Nov 11, 2014 at 2:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> +static int tcf_vlan_init(struct net *net, struct nlattr *nla,
>> + struct nlattr *est, struct tc_action *a,
>> + int ovr, int bind)
>> +{
>> + struct nlattr *tb[TCA_VLAN_MAX + 1];
>> + struct tc_vlan *parm;
>> + struct tcf_vlan *v;
>> + int action;
>> + __be16 push_vid = 0;
>> + __be16 push_proto = 0;
>> + int ret = 0;
>> + int err;
>> +
>> + if (!nla)
>> + return -EINVAL;
>> +
>> + err = nla_parse_nested(tb, TCA_VLAN_MAX, nla, vlan_policy);
>> + if (err < 0)
>> + return err;
>> +
>> + if (!tb[TCA_VLAN_PARMS])
>> + return -EINVAL;
>> + parm = nla_data(tb[TCA_VLAN_PARMS]);
>> + switch (parm->v_action) {
>> + case TCA_VLAN_ACT_POP:
>> + break;
>> + case TCA_VLAN_ACT_PUSH:
>> + if (!tb[TCA_VLAN_PUSH_VLAN_ID])
>> + return -EINVAL;
>> + push_vid = nla_get_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
>
>nla_get_be16() ?
No. I made this the same as it is done in net/8021q/vlan_netlink.c
>
>
>> + if (push_vid >= VLAN_VID_MASK)
>> + return -ERANGE;
>> +
>
>htons() ?
>
>[...]
>
>> +static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
>> + int bind, int ref)
>> +{
>> + unsigned char *b = skb_tail_pointer(skb);
>> + struct tcf_vlan *v = a->priv;
>> + struct tc_vlan opt = {
>> + .index = v->tcf_index,
>> + .refcnt = v->tcf_refcnt - ref,
>> + .bindcnt = v->tcf_bindcnt - bind,
>> + .action = v->tcf_action,
>> + .v_action = v->tcfv_action,
>> + };
>> + struct tcf_t t;
>> +
>> + if (nla_put(skb, TCA_VLAN_PARMS, sizeof(opt), &opt))
>> + goto nla_put_failure;
>> +
>> + if (v->tcfv_action == TCA_VLAN_ACT_PUSH &&
>> + nla_put_u16(skb, TCA_VLAN_PUSH_VLAN_ID, v->tcfv_push_vid) &&
>> + nla_put_u16(skb, TCA_VLAN_PUSH_VLAN_PROTOCOL, v->tcfv_push_proto))
>> + goto nla_put_failure;
>
>nla_put_be16() ?
^ permalink raw reply
* Re: [patch net-next 2/2] sched: introduce vlan action
From: Jamal Hadi Salim @ 2014-11-12 13:03 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang
Cc: netdev, David Miller, Pravin B Shelar, Tom Herbert, Eric Dumazet,
willemb, Daniel Borkmann, mst, Florian Westphal, Paul.Durrant,
Thomas Graf
In-Reply-To: <20141112123445.GE1882@nanopsycho.orion>
On 11/12/14 07:34, Jiri Pirko wrote:
> Tue, Nov 11, 2014 at 11:33:21PM CET, cwang@twopensource.com wrote:
>> On Tue, Nov 11, 2014 at 2:13 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> +static int tcf_vlan_init(struct net *net, struct nlattr *nla,
>>> + struct nlattr *est, struct tc_action *a,
>>> + int ovr, int bind)
>>> +{
>>> + struct nlattr *tb[TCA_VLAN_MAX + 1];
>>> + struct tc_vlan *parm;
>>> + struct tcf_vlan *v;
>>> + int action;
>>> + __be16 push_vid = 0;
>>> + __be16 push_proto = 0;
>>> + int ret = 0;
>>> + int err;
>>> +
>>> + if (!nla)
>>> + return -EINVAL;
>>> +
>>> + err = nla_parse_nested(tb, TCA_VLAN_MAX, nla, vlan_policy);
>>> + if (err < 0)
>>> + return err;
>>> +
>>> + if (!tb[TCA_VLAN_PARMS])
>>> + return -EINVAL;
>>> + parm = nla_data(tb[TCA_VLAN_PARMS]);
>>> + switch (parm->v_action) {
>>> + case TCA_VLAN_ACT_POP:
>>> + break;
>>> + case TCA_VLAN_ACT_PUSH:
>>> + if (!tb[TCA_VLAN_PUSH_VLAN_ID])
>>> + return -EINVAL;
>>> + push_vid = nla_get_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
>>
>> nla_get_be16() ?
>
> No. I made this the same as it is done in net/8021q/vlan_netlink.c
>
It just happens that user space passes it to you in BE already.
So this works out because skb_vlan_push expects that to be in BE
as in:
int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next 2/2] sched: introduce vlan action
From: Jamal Hadi Salim @ 2014-11-12 13:06 UTC (permalink / raw)
To: Jiri Pirko, Cong Wang
Cc: netdev, David Miller, Pravin B Shelar, Tom Herbert, Eric Dumazet,
willemb, Daniel Borkmann, mst, Florian Westphal, Paul.Durrant,
Thomas Graf
In-Reply-To: <54635AAC.3080600@mojatatu.com>
On 11/12/14 08:03, Jamal Hadi Salim wrote:
> On 11/12/14 07:34, Jiri Pirko wrote:
> It just happens that user space passes it to you in BE already.
> So this works out because skb_vlan_push expects that to be in BE
> as in:
> int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
>
Sorry, meant proto is in BE but vlan is in host order..
(grr.. need to test).
cheers,
jamal
^ permalink raw reply
* Device Tree Binding for Marvell DSA Switch on imx28 board over Mdio Interface
From: Oliver Graute @ 2014-11-12 13:07 UTC (permalink / raw)
To: netdev
Hello,
how do I specify the DSA node and the MDIO node in the Device Tree
Binding to integrate a Marvell 88e6071 switch with a imx28 board?
On my board the Marvell switch 88e6071 is connected via phy1 (on a
imx28 PCB) to phy5 on the Marvell switch (on a Switch PCB). All phys
are connected via the same MDIO Bus.
I enabled the Marvell DSA Support Driver, Gianfar Ethernet Driver and
Freescale PQ MDIO Driver in the Kernel (I' am not sure if this is the
right choice for imx28 fec ethernet controller is it?)
I already know that I need to adapt the DSA driver for this new
switch. But currently I can't access the switch ports because my MDIO
Bus is not configured correctly. It always ends with:
dmesg | grep -E "mii|dsa|mdio"
[ 2.528900] libphy: fec_enet_mii_bus: probed
[ 3.028061] !!!!Enter dsa Probe!!!!!
[ 3.037640] !!!!!Enter dsa_of_probe!!!!!
[ 3.041736] !!!!before of_parse_phandle dsa,mii-bus!!!!!
[ 3.047123] !!!! mdio->name=ethernet-phy !!!!!
[ 3.051658] !!!!before of_mdio_find_bus!!!!!
[ 3.055950] !!!!!enter of_mdio_find_bus!!!!!
[ 3.074074] !!!!!enter of_mdio_bus_match!!!!!
[ 3.078451] !!!!!enter of_mdio_bus_match!!!!!
[ 3.088915] !!!!Leave of_mdio_find_bus !!!!!
[ 3.093268] !!!! return of_mdio_find_bus =22 !!!!!
[ 3.098166] dsa_of_probe returns=-22
[ 3.101858] dsa: probe of dsa.5 failed with error -22
[ 19.169423] fec 800f0000.ethernet eth0: Freescale FEC PHY driver
[Micrel KSZ8041] (mii_bus:phy_addr=800f0000.etherne:00, irq=-1)
[ 20.038786] fec 800f4000.ethernet eth1: Freescale FEC PHY driver
[Micrel KSZ8041] (mii_bus:phy_addr=800f0000.etherne:01, irq=-1)
because the method of_mdio_find_bus returns with EINVAL
I suspect that the problem is the fact that the Kernel is not getting
connected to the MDIO bus.
what is wrong here?
dsa@0 {
compatible = "marvell,dsa";
#address-cells = <2>;
#size-cells = <0>;
interrupts = <10>;
dsa,ethernet = <ð1>;
dsa,mii-bus = <ðphy1>;
switch@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <5 0>; /* MDIO address 5, switch 0 in tree */
port@0 {
reg = <0>;
label = "lan1";
phy-handle = <ðphy1>;
};
port@1 {
reg = <1>;
label = "lan2";
};
port@2 {
reg = <2>;
label = "lan3";
};
port@3 {
reg = <3>;
label = "lan4";
};
port@4 {
reg = <4>;
label = "lan5";
};
port@5 {
reg = <5>;
label = "cpu";
};
};
};
eth1: eth1 {
status = "okay";
ethernet1-port@1 {
phy-handle = <ðphy1>;
};
};
mdio_bus: mdio {
#address-cells = <1>;
#size-cells = <0>;
device_type = "mdio";
//compatible = "fsl,gianfar-mdio";
compatible = "fsl,mpc875-fec-mdio", "fsl,pq1-fec-mdio";
reg = <0xe00 0x188>;
status = "okay";
ethphy0: ethernet-phy@0 {
reg = <0>;
};
ethphy1: ethernet-phy@1 {
reg = <1>;
};
//reg = <0xff>; */ /* No PHY attached */
//speed = <1000>;
//duple = <1>;
};
Best regards,
Oliver Graute
^ permalink raw reply
* Re: [PATCH net-next 0/3] dev_disable_lro() improvements for stacked devices
From: Michal Kubecek @ 2014-11-12 13:15 UTC (permalink / raw)
To: David Miller; +Cc: vfalico, netdev, linux-kernel, j.vosburgh, andy, jiri
In-Reply-To: <20141111.214752.266800170368088905.davem@davemloft.net>
On Tue, Nov 11, 2014 at 09:47:52PM -0500, David Miller wrote:
>
> Please do it generically.
>
> Having a special stanza for each layered device type in
> dev_disable_lro() is beyond stupid. Especially when it
> can in fact be done cleanly.
I gave it some thought and I would like ask one question first:
Does the upper-lower relationship always mean that upper device receives
packets through its lower device(s) so that LRO should always be
disabled for lower devices whenever there are some? Or should it be
limited only to an explicit list of device types (vlan, macvlan, bond,
team)?
Michal Kubecek
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox