* [PATCH net-next 0/5] fix stretch ACK bugs in TCP CUBIC and Reno
From: Neal Cardwell @ 2015-01-27 20:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell
This patch series fixes the TCP CUBIC and Reno congestion control
modules to properly handle stretch ACKs in their respective additive
increase modes, and in the transitions from slow start to additive
increase.
This finishes the project started by commit 9f9843a751d0a2057 ("tcp:
properly handle stretch acks in slow start"), which fixed behavior for
TCP congestion control when handling stretch ACKs in slow start mode.
Motivation: In the Jan 2015 netdev thread 'BW regression after "tcp:
refine TSO autosizing"', Eyal Perry documented a regression that Eric
Dumazet determined was caused by improper handling of TCP stretch
ACKs.
Background: LRO, GRO, delayed ACKs, and middleboxes can cause "stretch
ACKs" that cover more than the RFC-specified maximum of 2
packets. These stretch ACKs can cause serious performance shortfalls
in common congestion control algorithms, like Reno and CUBIC, which
were designed and tuned years ago with receiver hosts that were not
using LRO or GRO, and were instead ACKing every other packet.
Testing: at Google we have been using this approach for handling
stretch ACKs for CUBIC datacenter and Internet traffic for several
years, with good results.
Neal Cardwell (5):
tcp: stretch ACK fixes prep
tcp: fix the timid additive increase on stretch ACKs
tcp: fix stretch ACK bugs in Reno
tcp: fix stretch ACK bugs in CUBIC
tcp: fix timing issue in CUBIC slope calculation
include/net/tcp.h | 4 ++--
net/ipv4/tcp_bic.c | 2 +-
net/ipv4/tcp_cong.c | 32 ++++++++++++++++++++------------
net/ipv4/tcp_cubic.c | 38 +++++++++++++++++---------------------
net/ipv4/tcp_scalable.c | 3 ++-
net/ipv4/tcp_veno.c | 2 +-
net/ipv4/tcp_yeah.c | 2 +-
7 files changed, 44 insertions(+), 39 deletions(-)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply
* Loan Offer
From: Frank Moses @ 2015-01-27 13:04 UTC (permalink / raw)
To: Recipients
I am Frank Moses a private lender located in USA.I can help you with a loan you are looking for @ 3% interest rate,For Urgent Response Email: frankloanmoses@gmail.com
Frank Moses.
Email: frankloanmoses@gmail.com
Telephone:(781)-369-5127
^ permalink raw reply
* Re: [PATCH 2/2] net wireless wcn36xx adapt wcnss platform to select module by DT
From: Eugene Krasnikov @ 2015-01-27 20:27 UTC (permalink / raw)
To: Andy Green; +Cc: Kalle Valo, wcn36xx, linux-wireless, netdev
In-Reply-To: <CAAfg0W6TWxi7Lp_nZZ7peuHvuYfBckeqRQaeo42snG=z2pMeag@mail.gmail.com>
What i mean is that it's not clear who knows what chip is this,
whether wcn36xx or wcn36xx_msm. Previously the assumption was that SMD
command will tell what interface to use. Now we are moving towards
wcn36xx_msm telling what chip is installed. Both approaches will work.
If it less work to do then fine.
Sorry for any confusion.
2015-01-19 9:34 GMT+00:00 Andy Green <andy.green@linaro.org>:
> On 19 January 2015 at 17:02, Eugene Krasnikov <k.eugene.e@gmail.com> wrote:
>> The idea is definitely better than just checking for AC support. But
>> the question is whether platform/hardware/firmware support that?
>
> Sorry I don't understand what might be unsupported.
>
> - We don't ask the firmware, we tell the driver what chip it is from
> the outside. There's nothing for the firmware to support.
>
> - Platform supports a set of ops via platform_data already. This
> just adds one op to get the chip type from the platform code.
>
> - What can't the hardware support? The hardware physically is a
> 3620, 3660 or 3680. We just tell the driver what it is when we
> instantiate the device. We don't ask anything of the hardware.
>
> I expected to have a debate about whether to move the dt support to
> wcn36xx directly which is also reasonable... there's no question
> adding an op will work or not, it will work for all cases like this.
> But it also implies there must be the "device faking" business in -msm
> code, one day that will also go upstream and then it might be
> considered a bit strange.
>
> I did it like this now because it's the minimum change from the
> current situation.
>
> -Andy
>
>> 2015-01-19 9:00 GMT+00:00 Andy Green <andy.green@linaro.org>:
>>> On 19 January 2015 at 16:49, Eugene Krasnikov <k.eugene.e@gmail.com> wrote:
>>>> Have you tested this code on any device other than wcn3620?
>>>
>>> No... the only hardware I have is 3620. But the only code we're
>>> adding to mainline is the patch with the ops to get the chip type.
>>>
>>> The two-patch series just shows one way to set that (which will
>>> certainly work for all three defined compatible types, if it works for
>>> one). And this code cannot go upstream.
>>>
>>> So the only decision to make is around whether adding the platform op
>>> is a good way (or, eg, directly add DT support to wcn36xx and
>>> eliminate the 'device regeneration' part of the OOT -msm code).
>>>
>>> At the moment the detect code does not work for 3620, so something
>>> needs to be done.
>>>
>>> -Andy
>>>
>>>> 2015-01-19 8:44 GMT+00:00 Andy Green <andy.green@linaro.org>:
>>>>> On 19 January 2015 at 16:34, Eugene Krasnikov <k.eugene.e@gmail.com> wrote:
>>>>>
>>>>>> So how do we insmod wcn36xx_msm with a parameter specifying what type
>>>>>> of hardware do we use?
>>>>>
>>>>> The type of chip is defined in the DT "compatible" which also delivers
>>>>> the resource information.
>>>>>
>>>>> qcom,wcn36xx@0a000000 {
>>>>> compatible = "qcom,wcn3620";
>>>>> reg = <0x0a000000 0x280000>;
>>>>> reg-names = "wcnss_mmio";
>>>>>
>>>>> interrupts = <0 145 0 0 146 0>;
>>>>> interrupt-names = "wcnss_wlantx_irq", "wcnss_wlanrx_irq";
>>>>> ...
>>>>>
>>>>> This bit based on your code can't go in mainline until there's some
>>>>> kind of PIL support.
>>>>>
>>>>> So the only things we can discuss about it for mainline purpose is
>>>>> whether using a platform ops is a good way to interface to the
>>>>> mainline driver.
>>>>>
>>>>> If you're OK with that and you want a module parameter then this can
>>>>> grow a module parameter and prefer to deliver the chip type from that
>>>>> if given, without modifying the platform op interface.
>>>>>
>>>>> But with or without a module parameter this can't be upstreamed right
>>>>> now due to PIL.
>>>>>
>>>>> -Andy
>>>>>
>>>>>> 2015-01-18 5:16 GMT+00:00 Andy Green <andy.green@linaro.org>:
>>>>>>> Simplify the resource handling and use DT to indicate which chip type
>>>>>>> we are dealing with
>>>>>>>
>>>>>>> Signed-off-by: Andy Green <andy.green@linaro.org>
>>>>>>> ---
>>>>>>> drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c | 101 ++++++++++++------------
>>>>>>> 1 file changed, 52 insertions(+), 49 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c b/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c
>>>>>>> index f6f6c83..c9250e0 100644
>>>>>>> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c
>>>>>>> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx-msm.c
>>>>>>> @@ -42,7 +42,10 @@ struct wcn36xx_msm {
>>>>>>> struct completion smd_compl;
>>>>>>> smd_channel_t *smd_ch;
>>>>>>> struct pinctrl *pinctrl;
>>>>>>> -} wmsm;
>>>>>>> + enum wcn36xx_chip_type chip_type;
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct wcn36xx_msm wmsm;
>>>>>>>
>>>>>>> static int wcn36xx_msm_smsm_change_state(u32 clear_mask, u32 set_mask)
>>>>>>> {
>>>>>>> @@ -217,14 +220,47 @@ int wcn36xx_msm_powerup(const struct subsys_desc *desc)
>>>>>>> return 0;
>>>>>>> }
>>>>>>>
>>>>>>> +static const struct of_device_id wcn36xx_msm_match_table[] = {
>>>>>>> + { .compatible = "qcom,wcn3660", .data = (void *)WCN36XX_CHIP_3660 },
>>>>>>> + { .compatible = "qcom,wcn3680", .data = (void *)WCN36XX_CHIP_3680 },
>>>>>>> + { .compatible = "qcom,wcn3620", .data = (void *)WCN36XX_CHIP_3620 },
>>>>>>> + { }
>>>>>>> +};
>>>>>>> +
>>>>>>> +static int wcn36xx_msm_get_chip_type(void)
>>>>>>> +{
>>>>>>> + return wmsm.chip_type;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static struct wcn36xx_msm wmsm = {
>>>>>>> + .ctrl_ops = {
>>>>>>> + .open = wcn36xx_msm_smd_open,
>>>>>>> + .close = wcn36xx_msm_smd_close,
>>>>>>> + .tx = wcn36xx_msm_smd_send_and_wait,
>>>>>>> + .get_hw_mac = wcn36xx_msm_get_hw_mac,
>>>>>>> + .smsm_change_state = wcn36xx_msm_smsm_change_state,
>>>>>>> + .get_chip_type = wcn36xx_msm_get_chip_type,
>>>>>>> + },
>>>>>>> +};
>>>>>>> +
>>>>>>> static int wcn36xx_msm_probe(struct platform_device *pdev)
>>>>>>> {
>>>>>>> int ret;
>>>>>>> - struct resource *wcnss_memory;
>>>>>>> - struct resource *tx_irq;
>>>>>>> - struct resource *rx_irq;
>>>>>>> + const struct of_device_id *of_id;
>>>>>>> + struct resource *r;
>>>>>>> struct resource res[3];
>>>>>>> struct pinctrl_state *ps;
>>>>>>> + static const char const *rnames[] = {
>>>>>>> + "wcnss_mmio", "wcnss_wlantx_irq", "wcnss_wlanrx_irq" };
>>>>>>> + static const int rtype[] = {
>>>>>>> + IORESOURCE_MEM, IORESOURCE_IRQ, IORESOURCE_IRQ };
>>>>>>> + int n;
>>>>>>> +
>>>>>>> + of_id = of_match_node(wcn36xx_msm_match_table, pdev->dev.of_node);
>>>>>>> + if (!of_id)
>>>>>>> + return -EINVAL;
>>>>>>> +
>>>>>>> + wmsm.chip_type = (enum wcn36xx_chip_type)of_id->data;
>>>>>>>
>>>>>>> wmsm.pinctrl = devm_pinctrl_get(&pdev->dev);
>>>>>>> if (IS_ERR_OR_NULL(wmsm.pinctrl))
>>>>>>> @@ -240,52 +276,23 @@ static int wcn36xx_msm_probe(struct platform_device *pdev)
>>>>>>>
>>>>>>> if (IS_ERR_OR_NULL(pil))
>>>>>>> pil = subsystem_get("wcnss");
>>>>>>> - if (IS_ERR_OR_NULL(pil))
>>>>>>> - return PTR_ERR(pil);
>>>>>>> + if (IS_ERR_OR_NULL(pil))
>>>>>>> + return PTR_ERR(pil);
>>>>>>>
>>>>>>> wmsm.core = platform_device_alloc("wcn36xx", -1);
>>>>>>>
>>>>>>> - //dev_err(&pdev->dev, "%s starting\n", __func__);
>>>>>>> -
>>>>>>> - memset(res, 0x00, sizeof(res));
>>>>>>> - wmsm.ctrl_ops.open = wcn36xx_msm_smd_open;
>>>>>>> - wmsm.ctrl_ops.close = wcn36xx_msm_smd_close;
>>>>>>> - wmsm.ctrl_ops.tx = wcn36xx_msm_smd_send_and_wait;
>>>>>>> - wmsm.ctrl_ops.get_hw_mac = wcn36xx_msm_get_hw_mac;
>>>>>>> - wmsm.ctrl_ops.smsm_change_state = wcn36xx_msm_smsm_change_state;
>>>>>>> - wcnss_memory =
>>>>>>> - platform_get_resource_byname(pdev,
>>>>>>> - IORESOURCE_MEM,
>>>>>>> - "wcnss_mmio");
>>>>>>> - if (wcnss_memory == NULL) {
>>>>>>> - dev_err(&wmsm.core->dev,
>>>>>>> - "Failed to get wcnss wlan memory map.\n");
>>>>>>> - ret = -ENOMEM;
>>>>>>> - return ret;
>>>>>>> - }
>>>>>>> - memcpy(&res[0], wcnss_memory, sizeof(*wcnss_memory));
>>>>>>> -
>>>>>>> - tx_irq = platform_get_resource_byname(pdev,
>>>>>>> - IORESOURCE_IRQ,
>>>>>>> - "wcnss_wlantx_irq");
>>>>>>> - if (tx_irq == NULL) {
>>>>>>> - dev_err(&wmsm.core->dev, "Failed to get wcnss tx_irq");
>>>>>>> - ret = -ENOMEM;
>>>>>>> - return ret;
>>>>>>> - }
>>>>>>> - memcpy(&res[1], tx_irq, sizeof(*tx_irq));
>>>>>>> -
>>>>>>> - rx_irq = platform_get_resource_byname(pdev,
>>>>>>> - IORESOURCE_IRQ,
>>>>>>> - "wcnss_wlanrx_irq");
>>>>>>> - if (rx_irq == NULL) {
>>>>>>> - dev_err(&wmsm.core->dev, "Failed to get wcnss rx_irq");
>>>>>>> - ret = -ENOMEM;
>>>>>>> - return ret;
>>>>>>> + for (n = 0; n < ARRAY_SIZE(rnames); n++) {
>>>>>>> + r = platform_get_resource_byname(pdev, rtype[n], rnames[n]);
>>>>>>> + if (!r) {
>>>>>>> + dev_err(&wmsm.core->dev,
>>>>>>> + "Missing resource %s'\n", rnames[n]);
>>>>>>> + ret = -ENOMEM;
>>>>>>> + return ret;
>>>>>>> + }
>>>>>>> + res[n] = *r;
>>>>>>> }
>>>>>>> - memcpy(&res[2], rx_irq, sizeof(*rx_irq));
>>>>>>>
>>>>>>> - platform_device_add_resources(wmsm.core, res, ARRAY_SIZE(res));
>>>>>>> + platform_device_add_resources(wmsm.core, res, n);
>>>>>>>
>>>>>>> ret = platform_device_add_data(wmsm.core, &wmsm.ctrl_ops,
>>>>>>> sizeof(wmsm.ctrl_ops));
>>>>>>> @@ -319,10 +326,6 @@ static int wcn36xx_msm_remove(struct platform_device *pdev)
>>>>>>> return 0;
>>>>>>> }
>>>>>>>
>>>>>>> -static const struct of_device_id wcn36xx_msm_match_table[] = {
>>>>>>> - { .compatible = "qcom,wcn36xx" },
>>>>>>> - { }
>>>>>>> -};
>>>>>>> MODULE_DEVICE_TABLE(of, wcn36xx_msm_match_table);
>>>>>>>
>>>>>>> static struct platform_driver wcn36xx_msm_driver = {
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards,
>>>>>> Eugene
>>>>
>>>>
>>>>
>>>> --
>>>> Best regards,
>>>> Eugene
>>
>>
>>
>> --
>> Best regards,
>> Eugene
--
Best regards,
Eugene
^ permalink raw reply
* Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]
From: Alexander Aring @ 2015-01-27 20:26 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, davem, arvid.brodin, linux-wpan
In-Reply-To: <54C7A5B7.60103@6wind.com>
On Tue, Jan 27, 2015 at 03:50:31PM +0100, Nicolas Dichtel wrote:
> Le 27/01/2015 15:06, Alexander Aring a écrit :
> >Hi,
> >
> >On Tue, Jan 27, 2015 at 02:28:47PM +0100, Nicolas Dichtel wrote:
> >...
> [snip]
> >>
> >>I don't know how wpan0 is created and if this interface can be created directly
> >>in another netns than init_net.
> >>
> >
> >no it can't. The wpan0 interface can be created via the 802.15.4
> >userspace tools and we don't have such option for namespaces. It
> >should be always to init_net while creation.
> Even with 'ip netns exec foo iwpan ...'?
>
I did a quick test:
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 6fb6bdf..df55b42 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -590,6 +590,11 @@ ieee802154_if_add(struct ieee802154_local *local, const char *name,
list_add_tail_rcu(&sdata->list, &local->interfaces);
mutex_unlock(&local->iflist_mtx);
+ if (net_eq(dev_net(ndev), &init_net))
+ printk(KERN_INFO "it's init_net\n");
+ else
+ printk(KERN_INFO "it's not init_net\n");
With this patch and running:
ip netns exec foo iwpan phy phy0 interface add bar%d type node
I got a:
[ 52.032956] it's init_net
It's also init_net when I run with "ip netns exec foo iwpan ..."
> >
> >>>
> >>>
> >>>Summarize:
> >>>
> >>>I would add the dev->features |= NETIF_F_NETNS_LOCAL; while wpan
> >>>interface generation and add only the !net_eq(src_net, &init_net) check
> >>>above. I suppose that src_net is the net namespace from "underlaying"
> >>>interface wpan by calling:
> >>>
> >>>$ ip link add link wpan0 name lowpan0 type lowpan
> >>No. src_net is the netns where the ip command is launched. With this patch, my
> >
> >ah, and when no "ip netns" is given it's default to init_net?
> The default netns is the netns where your shell is running :)
Now, I understood how basically that works (I think).
> It may be different from init_net when you are playing on a virtual machine. On
> a physical machine, it's usually init_net.
>
ok.
> >
> >
> >Okay, then I agree with that both interfaces should be set
> >
> >dev->features |= NETIF_F_NETNS_LOCAL
> Ok.
>
> >
> >because both interfaces should started with "init_net" as default
> >namespace. For wpan interface this should always be in "init_net",
> >because we don't set anything while creation.
> Not sure this is true. It's probably possible to create it directly in another
> netns (with 'ip netns exec' or because your system is a virtual machine that
> runs over a namespaces construction (see docker [0], lxc [1], etc).
>
> [0] https://www.docker.com/
> [1] https://linuxcontainers.org/
>
ah, ok.
> >
> >For 6LoWPAN interface this should also always in the same namespace like
> >the wpan interface and not diffrent namespace between link (wpan) and
> >virtual (6LoWPAN) interface.
> >
> >Do you agree with that?
> Yes.
>
> But I still wonder if we should add a check about dev_net(dev) != init_net in
> net/ieee802154/6lowpan/core.c.
> If my understanding is correct:
> - wpan can be created directly in a netns != init_net
> - 6lowpan must be in the same netns than wpan
> - code under net/ieee802154 only works in init_net, thus 6lowpan only works
> in init_net.
>
> Do you agree?
yes. I will put the last item on my ToDo list if we can do more netns
stuff there.
> What about this (based on net-next)?
>
There are some little issues, see below.
> From 5ca1c46c68e4e4381b2f7e284f5dadeb28a53b2f Mon Sep 17 00:00:00 2001
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Tue, 27 Jan 2015 11:26:20 +0100
> Subject: [PATCH] wpan/6lowpan: fix netns settings
>
use "ieee802154:" instead "wpan/6lowpan:".
Can you rebase this patch on bluetooth-next?
> 6LoWPAN currently doesn't supports x-netns and works only in init_net.
>
> With this patch, we ensure that:
> - the wpan interface cannot be moved to another netns;
> - the 6lowpan interface cannot be moved to another netns;
> - the wpan interface is in the same netns than the 6lowpan interface;
> - the 6lowpan interface is in init_net.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> net/ieee802154/6lowpan/core.c | 6 ++++--
> net/mac802154/iface.c | 1 +
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 055fbb71ba6f..dfd3c6007f60 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -126,6 +126,7 @@ static void lowpan_setup(struct net_device *dev)
> dev->header_ops = &lowpan_header_ops;
> dev->ml_priv = &lowpan_mlme;
> dev->destructor = free_netdev;
> + dev->features |= NETIF_F_NETNS_LOCAL;
> }
>
> static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
> @@ -148,10 +149,11 @@ static int lowpan_newlink(struct net *src_net, struct
> net_device *dev,
>
> pr_debug("adding new link\n");
>
> - if (!tb[IFLA_LINK])
> + if (!tb[IFLA_LINK] ||
> + !net_eq(dev_net(dev), &init_net))
> return -EINVAL;
> /* find and hold real wpan device */
> - real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
> + real_dev = dev_get_by_index(dev_net(dev), nla_get_u32(tb[IFLA_LINK]));
> if (!real_dev)
> return -ENODEV;
> if (real_dev->type != ARPHRD_IEEE802154) {
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 6fb6bdf9868c..b67da8d578b4 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -475,6 +475,7 @@ static void ieee802154_if_setup(struct net_device *dev)
> dev->mtu = IEEE802154_MTU;
> dev->tx_queue_len = 300;
> dev->flags = IFF_NOARP | IFF_BROADCAST;
> + dev->features |= NETIF_F_NETNS_LOCAL;
We should set this inside the cfg802154_netdev_notifier_call function
and NETDEV_REGISTER case. This can be found in "net/ieee802154/core.c". [0]
The branch "net/mac802154" affects 802.15.4 SoftMAC interfaces only. We
currently support SoftMAC only, but further we support HardMAC drivers.
The HardMAC drivers doesn't use the "net/mac802154" branch and call
netdev_register in driver layer.
When we do it in cfg802154_netdev_notifier_call then this will be set
for SoftMAC and HardMAC drivers (when we have a HardMAC driver). The
same behaviour can also be found in wireless implementation. [1]
- Alex
[0] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/ieee802154/core.c#n227
[1] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/wireless/core.c#n1000
^ permalink raw reply related
* [PATCH] net: don't OOPS on socket aio
From: David Miller @ 2015-01-27 20:26 UTC (permalink / raw)
To: netdev; +Cc: hch
From: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/socket.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index a2c33a4..418795c 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -869,9 +869,6 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
static struct sock_iocb *alloc_sock_iocb(struct kiocb *iocb,
struct sock_iocb *siocb)
{
- if (!is_sync_kiocb(iocb))
- BUG();
-
siocb->kiocb = iocb;
iocb->private = siocb;
return siocb;
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH v1] stmmac: prevent probe drivers to crash kernel
From: David Miller @ 2015-01-27 20:24 UTC (permalink / raw)
To: andriy.shevchenko; +Cc: peppe.cavallaro, netdev
In-Reply-To: <1422376683-30555-1-git-send-email-andriy.shevchenko@linux.intel.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Tue, 27 Jan 2015 18:38:03 +0200
> In the case when alloc_netdev fails we return NULL to a caller. But there is no
> check for NULL in the probe drivers. This patch changes NULL to an error
> pointer. The function description is amended to reflect what we may get
> returned.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied, thank you.
^ permalink raw reply
* Re: Interaction between GSO and TSQ for single TCP stream
From: Tom Herbert @ 2015-01-27 20:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Neal Cardwell, Yuchung Cheng, Linux Netdev List
In-Reply-To: <CANn89iJCOuQfixdSp6TsPxpyvTxQSTLBJLgnsvbrbG2Am_p0-Q@mail.gmail.com>
On Tue, Jan 27, 2015 at 12:02 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Tue, Jan 27, 2015 at 11:57 AM, Tom Herbert <therbert@google.com> wrote:
>> Eric,
>>
>> I am looking at why we are unable to achieve line rate throughput with
>> a single connection using UDP encapsulation that supports GSO/GRO. I
>> discovered that even for a non-encpasulated TCP connection I'm not
>> able to get line rate using GSO/GRO (but do with TSO/LRO). There seems
>> to be some interaction between TSQ and GSO. Setting
>> tcp_limit_output_bytes to 1000000 gets single connection up to line
>> rate for both native and encapsulated TCP.
>>
>> Some data:
>>
>> - netperf TCP_STREAM for a single connection no encapsulation
>> Using TSO/LRO
>> 9412.44 Gbps
>> Using GSO/GRO (ethtool -K eth0 gso on gro on tso off lro off)
>> 9286.08 Gbps
>> Using GSO/GRO with larger limit (echo 1000000 >
>> /proc/sys/net/ipv4/tcp_limit_output_bytes)
>> 9412.75
>>
>> - netperf TCP_STREAM for a single connection for GUE-IPIP encapsulation with RCO
>> Using GSO/GRO
>> 8842.89 Gbps
>> Using GSO/GRO with larger limit (echo 1000000 >
>> /proc/sys/net/ipv4/tcp_limit_output_bytes)
>> 9143.99 Gbps
>>
>> Any ideas on how to address this?
>>
>> Thanks,
>> Tom
>
> Hi Tom.
>
> Yes, Eyal Perry reported the issue a while back, we plan to send
> upstream patches to fix the issue today.
>
Awesome, looking forward to seeing them!
> Make sure you tweaked coalescing parameters if you use mlx4 :
>
> ethtool -C eth0 tx-usecs 4 tx-frames 4
>
It's bnx2x.
> Otherwise, the NIC accumulates too many packets before sending an
> interrupt for TX completion.
>
> Then you can either wait the patches, or try the one I sent :
> http://permalink.gmane.org/gmane.linux.network/347023
^ permalink raw reply
* Re: Interaction between GSO and TSQ for single TCP stream
From: Eric Dumazet @ 2015-01-27 20:02 UTC (permalink / raw)
To: Tom Herbert, Neal Cardwell, Yuchung Cheng; +Cc: Linux Netdev List
In-Reply-To: <CA+mtBx8QSg+3VSiDgY6iqiJaQgScSd9FP-uxH8S5zGGv7tN1Yw@mail.gmail.com>
On Tue, Jan 27, 2015 at 11:57 AM, Tom Herbert <therbert@google.com> wrote:
> Eric,
>
> I am looking at why we are unable to achieve line rate throughput with
> a single connection using UDP encapsulation that supports GSO/GRO. I
> discovered that even for a non-encpasulated TCP connection I'm not
> able to get line rate using GSO/GRO (but do with TSO/LRO). There seems
> to be some interaction between TSQ and GSO. Setting
> tcp_limit_output_bytes to 1000000 gets single connection up to line
> rate for both native and encapsulated TCP.
>
> Some data:
>
> - netperf TCP_STREAM for a single connection no encapsulation
> Using TSO/LRO
> 9412.44 Gbps
> Using GSO/GRO (ethtool -K eth0 gso on gro on tso off lro off)
> 9286.08 Gbps
> Using GSO/GRO with larger limit (echo 1000000 >
> /proc/sys/net/ipv4/tcp_limit_output_bytes)
> 9412.75
>
> - netperf TCP_STREAM for a single connection for GUE-IPIP encapsulation with RCO
> Using GSO/GRO
> 8842.89 Gbps
> Using GSO/GRO with larger limit (echo 1000000 >
> /proc/sys/net/ipv4/tcp_limit_output_bytes)
> 9143.99 Gbps
>
> Any ideas on how to address this?
>
> Thanks,
> Tom
Hi Tom.
Yes, Eyal Perry reported the issue a while back, we plan to send
upstream patches to fix the issue today.
Make sure you tweaked coalescing parameters if you use mlx4 :
ethtool -C eth0 tx-usecs 4 tx-frames 4
Otherwise, the NIC accumulates too many packets before sending an
interrupt for TX completion.
Then you can either wait the patches, or try the one I sent :
http://permalink.gmane.org/gmane.linux.network/347023
^ permalink raw reply
* Re: [PATCH 4/7] net: wireless: wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND
From: Eugene Krasnikov @ 2015-01-27 20:01 UTC (permalink / raw)
To: Andy Green; +Cc: Kalle Valo, wcn36xx, linux-wireless, netdev
In-Reply-To: <20150118051100.31866.37997.stgit@114-36-241-182.dynamic.hinet.net>
Do you know when is this message used? sounds important.
2015-01-18 5:11 GMT+00:00 Andy Green <andy.green@linaro.org>:
> WCN3620 firmware introduces a new async indication, we need to
> add it as a known message type so we can accept it
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
> drivers/net/wireless/ath/wcn36xx/hal.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
> index a1f1127..b947de0 100644
> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
> @@ -345,6 +345,8 @@ enum wcn36xx_hal_host_msg_type {
> WCN36XX_HAL_DHCP_START_IND = 189,
> WCN36XX_HAL_DHCP_STOP_IND = 190,
>
> + WCN36XX_HAL_AVOID_FREQ_RANGE_IND = 233,
> +
> WCN36XX_HAL_MSG_MAX = WCN36XX_HAL_MSG_TYPE_MAX_ENUM_SIZE
> };
>
>
--
Best regards,
Eugene
^ permalink raw reply
* Re: [PATCH 7/7] net: wireless: wcn36xx: handle new trigger_ba format
From: Eugene Krasnikov @ 2015-01-27 19:58 UTC (permalink / raw)
To: Andy Green; +Cc: Kalle Valo, wcn36xx, linux-wireless, netdev
In-Reply-To: <20150118051117.31866.68634.stgit@114-36-241-182.dynamic.hinet.net>
arg... it looks like the code is starting to have to many
if(chip_version) cases. Mabe we should concider separate files for
chip specific logic.
2015-01-18 5:11 GMT+00:00 Andy Green <andy.green@linaro.org>:
> wcn3620 has a new message structure for the reply to trigger_ba
> We don't know what to do with the candidate list he sends back,
> but we can at least accept and ignore it nicely instead of dying.
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
> drivers/net/wireless/ath/wcn36xx/smd.c | 28 ++++++++++++++++++++++++++--
> drivers/net/wireless/ath/wcn36xx/smd.h | 9 +++++++++
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 819741c..dc24e1b 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -243,8 +243,31 @@ static int wcn36xx_smd_rsp_status_check(void *buf, size_t len)
> rsp = (struct wcn36xx_fw_msg_status_rsp *)
> (buf + sizeof(struct wcn36xx_hal_msg_header));
>
> - if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
> + if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
> + pr_err("%s: bad status, len = %d\n", __func__, len);
> + return rsp->status;
> + }
> +
> + return 0;
> +}
> +
> +static int wcn36xx_smd_rsp_status_check_bav2(struct wcn36xx *wcn, void *buf,
> + size_t len)
> +{
> + struct wcn36xx_fw_msg_status_rspv2 *rsp;
> +
> + if (wcn->chip_version != WCN36XX_CHIP_3620)
> + return wcn36xx_smd_rsp_status_check(buf, len);
> +
> + if (len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
> + return -EIO;
> +
> + rsp = buf + sizeof(struct wcn36xx_hal_msg_header);
> +
> + if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
> + pr_err("%s: bad status, len = %d\n", __func__, len);
> return rsp->status;
> + }
>
> return 0;
> }
> @@ -1884,7 +1907,8 @@ int wcn36xx_smd_trigger_ba(struct wcn36xx *wcn, u8 sta_index)
> wcn36xx_err("Sending hal_trigger_ba failed\n");
> goto out;
> }
> - ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
> + ret = wcn36xx_smd_rsp_status_check_bav2(wcn, wcn->hal_buf,
> + wcn->hal_rsp_len);
> if (ret) {
> wcn36xx_err("hal_trigger_ba response failed err=%d\n", ret);
> goto out;
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
> index 008d034..432d3b8 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.h
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.h
> @@ -44,6 +44,15 @@ struct wcn36xx_fw_msg_status_rsp {
> u32 status;
> } __packed;
>
> +/* wcn3620 returns this for tigger_ba */
> +
> +struct wcn36xx_fw_msg_status_rspv2 {
> + u8 bss_id[6];
> + u32 status __packed;
> + u16 count_following_candidates __packed;
> + /* candidate list follows */
> +};
> +
> struct wcn36xx_hal_ind_msg {
> struct list_head list;
> u8 *msg;
>
--
Best regards,
Eugene
^ permalink raw reply
* Interaction between GSO and TSQ for single TCP stream
From: Tom Herbert @ 2015-01-27 19:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Netdev List
Eric,
I am looking at why we are unable to achieve line rate throughput with
a single connection using UDP encapsulation that supports GSO/GRO. I
discovered that even for a non-encpasulated TCP connection I'm not
able to get line rate using GSO/GRO (but do with TSO/LRO). There seems
to be some interaction between TSQ and GSO. Setting
tcp_limit_output_bytes to 1000000 gets single connection up to line
rate for both native and encapsulated TCP.
Some data:
- netperf TCP_STREAM for a single connection no encapsulation
Using TSO/LRO
9412.44 Gbps
Using GSO/GRO (ethtool -K eth0 gso on gro on tso off lro off)
9286.08 Gbps
Using GSO/GRO with larger limit (echo 1000000 >
/proc/sys/net/ipv4/tcp_limit_output_bytes)
9412.75
- netperf TCP_STREAM for a single connection for GUE-IPIP encapsulation with RCO
Using GSO/GRO
8842.89 Gbps
Using GSO/GRO with larger limit (echo 1000000 >
/proc/sys/net/ipv4/tcp_limit_output_bytes)
9143.99 Gbps
Any ideas on how to address this?
Thanks,
Tom
^ permalink raw reply
* Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit
From: Jack Morgenstein @ 2015-01-27 19:50 UTC (permalink / raw)
To: David Miller; +Cc: David.Laight, amirv, netdev, ogerlitz, yevgenyp
In-Reply-To: <20150127.111334.501141232585900738.davem@davemloft.net>
On Tue, 27 Jan 2015 11:13:34 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> Indeed, I'm really sick of seeing these packed structures being
> created all over the place.
>
> They are to be used in absolutely extreme cases where no other
> solution is possible.
The V2 of this patch does not have "packed".
-Jack
^ permalink raw reply
* [PATCH v3 2/3] stmmac: pci: introduce Intel Quark X1000 runtime detection
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, David S . Miller
Cc: Andy Shevchenko, Kweh, Hock Leong
In-Reply-To: <1422387889-21559-1-git-send-email-andriy.shevchenko@linux.intel.com>
This patch introduces run-time board detection through DMI and MAC-PHY
configuration function used by quark_default_data() during initialization. It
fills up the phy_addr for Galileo and Galileo Gen2 boards to indicate that the
Ethernet MAC controller is or is not connected to any PHY.
The implementation takes into consideration for future expansion in Quark
series boards that may have different PHY address that is linked to its MAC
controllers.
This piece of work is derived from Bryan O'Donoghue's initial work for Quark
X1000 enabling.
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 62 +++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index a316187..50f3c50 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -24,14 +24,50 @@
*******************************************************************************/
#include <linux/pci.h>
+#include <linux/dmi.h>
+
#include "stmmac.h"
+/*
+ * This struct is used to associate PCI Function of MAC controller on a board,
+ * discovered via DMI, with the address of PHY connected to the MAC. The
+ * negative value of the address means that MAC controller is not connected
+ * with PHY.
+ */
+struct stmmac_pci_dmi_data {
+ const char *name;
+ unsigned int func;
+ int phy_addr;
+};
+
struct stmmac_pci_info {
struct pci_dev *pdev;
int (*setup)(struct plat_stmmacenet_data *plat,
struct stmmac_pci_info *info);
+ struct stmmac_pci_dmi_data *dmi;
};
+static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
+{
+ const char *name = dmi_get_system_info(DMI_BOARD_NAME);
+ unsigned int func = PCI_FUNC(info->pdev->devfn);
+ struct stmmac_pci_dmi_data *dmi;
+
+ /*
+ * Galileo boards with old firmware don't support DMI. We always return
+ * 1 here, so at least first found MAC controller would be probed.
+ */
+ if (!name)
+ return 1;
+
+ for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
+ if (!strcmp(dmi->name, name) && dmi->func == func)
+ return dmi->phy_addr;
+ }
+
+ return -ENODEV;
+}
+
static void stmmac_default_data(struct plat_stmmacenet_data *plat)
{
plat->bus_id = 1;
@@ -58,9 +94,18 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
struct stmmac_pci_info *info)
{
struct pci_dev *pdev = info->pdev;
+ int ret;
+
+ /*
+ * Refuse to load the driver and register net device if MAC controller
+ * does not connect to any PHY interface.
+ */
+ ret = stmmac_pci_find_phy_addr(info);
+ if (ret < 0)
+ return ret;
plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
- plat->phy_addr = 1;
+ plat->phy_addr = ret;
plat->interface = PHY_INTERFACE_MODE_RMII;
plat->clk_csr = 2;
plat->has_gmac = 1;
@@ -82,8 +127,23 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
return 0;
}
+static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
+ {
+ .name = "Galileo",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {
+ .name = "GalileoGen2",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {}
+};
+
static struct stmmac_pci_info quark_pci_info = {
.setup = quark_default_data,
+ .dmi = quark_pci_dmi_data,
};
/**
--
2.1.4
^ permalink raw reply related
* [PATCH v3 3/3] stmmac: pci: add MSI support for Intel Quark X1000
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, David S . Miller
Cc: Kweh, Hock Leong, Andy Shevchenko
In-Reply-To: <1422387889-21559-1-git-send-email-andriy.shevchenko@linux.intel.com>
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
In Intel Quark SoC X1000, both of the Ethernet controllers support
MSI interrupt handling. This patch enables them to use MSI interrupt
servicing in stmmac_pci for Intel Quark X1000.
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 50f3c50..3bca908 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -212,6 +212,8 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
} else
stmmac_default_data(plat);
+ pci_enable_msi(pdev);
+
priv = stmmac_dvr_probe(&pdev->dev, plat, pcim_iomap_table(pdev)[i]);
if (IS_ERR(priv)) {
dev_err(&pdev->dev, "%s: main driver probe failed\n", __func__);
--
2.1.4
^ permalink raw reply related
* [PATCH v3 1/3] stmmac: pci: add support for Intel Quark X1000
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, David S . Miller
Cc: Kweh, Hock Leong, Andy Shevchenko
In-Reply-To: <1422387889-21559-1-git-send-email-andriy.shevchenko@linux.intel.com>
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
The Intel Quark SoC X1000 provides two 10/100 Mbps Ethernet MAC
controllers which may or may not be connected to PHY on board.
This MAC controller only supports RMII PHY. This patch add Quark
PCI ID as well as Quark default platform data info to this driver.
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 51 +++++++++++++++++++++++-
1 file changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 054520d..a316187 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -26,6 +26,12 @@
#include <linux/pci.h>
#include "stmmac.h"
+struct stmmac_pci_info {
+ struct pci_dev *pdev;
+ int (*setup)(struct plat_stmmacenet_data *plat,
+ struct stmmac_pci_info *info);
+};
+
static void stmmac_default_data(struct plat_stmmacenet_data *plat)
{
plat->bus_id = 1;
@@ -48,6 +54,38 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
plat->unicast_filter_entries = 1;
}
+static int quark_default_data(struct plat_stmmacenet_data *plat,
+ struct stmmac_pci_info *info)
+{
+ struct pci_dev *pdev = info->pdev;
+
+ plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
+ plat->phy_addr = 1;
+ plat->interface = PHY_INTERFACE_MODE_RMII;
+ plat->clk_csr = 2;
+ plat->has_gmac = 1;
+ plat->force_sf_dma_mode = 1;
+
+ plat->mdio_bus_data->phy_reset = NULL;
+ plat->mdio_bus_data->phy_mask = 0;
+
+ plat->dma_cfg->pbl = 16;
+ plat->dma_cfg->burst_len = DMA_AXI_BLEN_256;
+ plat->dma_cfg->fixed_burst = 1;
+
+ /* Set default value for multicast hash bins */
+ plat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+ /* Set default value for unicast filter entries */
+ plat->unicast_filter_entries = 1;
+
+ return 0;
+}
+
+static struct stmmac_pci_info quark_pci_info = {
+ .setup = quark_default_data,
+};
+
/**
* stmmac_pci_probe
*
@@ -63,6 +101,7 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
static int stmmac_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
+ struct stmmac_pci_info *info = (struct stmmac_pci_info *)id->driver_data;
struct plat_stmmacenet_data *plat;
struct stmmac_priv *priv;
int i;
@@ -103,7 +142,15 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
pci_set_master(pdev);
- stmmac_default_data(plat);
+ if (info) {
+ info->pdev = pdev;
+ if (info->setup) {
+ ret = info->setup(plat, info);
+ if (ret)
+ return ret;
+ }
+ } else
+ stmmac_default_data(plat);
priv = stmmac_dvr_probe(&pdev->dev, plat, pcim_iomap_table(pdev)[i]);
if (IS_ERR(priv)) {
@@ -155,11 +202,13 @@ static int stmmac_pci_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_pci_suspend, stmmac_pci_resume);
#define STMMAC_VENDOR_ID 0x700
+#define STMMAC_QUARK_ID 0x0937
#define STMMAC_DEVICE_ID 0x1108
static const struct pci_device_id stmmac_id_table[] = {
{PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)},
{PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)},
+ {PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info},
{}
};
--
2.1.4
^ permalink raw reply related
* [PATCH v3 0/3] net: stmmac: Enable Intel Quark SoC X1000 Ethernet support
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, David S . Miller; +Cc: Andy Shevchenko
This is third version of the patch series [1] to bring network card support to
Intel Quark SoC.
The series has been tested on Intel Galileo board.
Changelog v3:
- rebase on top of recent net-next
- rework an approach to get the custom configuration
- rework an approach how to get unique bus_id
- improve DMI lookup function
[1] http://www.spinics.net/lists/netdev/msg296010.html
Andy Shevchenko (1):
stmmac: pci: introduce Intel Quark X1000 runtime detection
Kweh, Hock Leong (2):
stmmac: pci: add support for Intel Quark X1000
stmmac: pci: add MSI support for Intel Quark X1000
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 113 ++++++++++++++++++++++-
1 file changed, 112 insertions(+), 1 deletion(-)
--
2.1.4
^ permalink raw reply
* [PATCH net-next V2 1/1] dev: add per net_device packet type chains
From: Salam Noureddine @ 2015-01-27 19:35 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Tom Herbert, Jiri Pirko,
Vlad Yasevich, netdev
Cc: Salam Noureddine, Eric W. Biederman
When many pf_packet listeners are created on a lot of interfaces the
current implementation using global packet type lists scales poorly.
This patch adds per net_device packet type lists to fix this problem.
The patch was originally written by Eric Biederman for linux-2.6.29.
Tested on linux-3.16.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Salam Noureddine <noureddine@arista.com>
---
include/linux/netdevice.h | 2 +
net/core/dev.c | 132 +++++++++++++++++++++++++++++-----------------
2 files changed, 86 insertions(+), 48 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 642d426..3d37c6e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1514,6 +1514,8 @@ struct net_device {
struct list_head napi_list;
struct list_head unreg_list;
struct list_head close_list;
+ struct list_head ptype_all;
+ struct list_head ptype_specific;
struct {
struct list_head upper;
diff --git a/net/core/dev.c b/net/core/dev.c
index 1e325ad..a7b6c62 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -371,9 +371,10 @@ static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
static inline struct list_head *ptype_head(const struct packet_type *pt)
{
if (pt->type == htons(ETH_P_ALL))
- return &ptype_all;
+ return pt->dev ? &pt->dev->ptype_all : &ptype_all;
else
- return &ptype_base[ntohs(pt->type) & PTYPE_HASH_MASK];
+ return pt->dev ? &pt->dev->ptype_specific :
+ &ptype_base[ntohs(pt->type) & PTYPE_HASH_MASK];
}
/**
@@ -1734,6 +1735,23 @@ static inline int deliver_skb(struct sk_buff *skb,
return pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
}
+static inline void deliver_ptype_list_skb(struct sk_buff *skb,
+ struct packet_type **pt,
+ struct net_device *dev, __be16 type,
+ struct list_head *ptype_list)
+{
+ struct packet_type *ptype, *pt_prev = *pt;
+
+ list_for_each_entry_rcu(ptype, ptype_list, list) {
+ if (ptype->type != type)
+ continue;
+ if (pt_prev)
+ deliver_skb(skb, pt_prev, dev);
+ pt_prev = ptype;
+ }
+ *pt = pt_prev;
+}
+
static inline bool skb_loop_sk(struct packet_type *ptype, struct sk_buff *skb)
{
if (!ptype->af_packet_priv || !skb->sk)
@@ -1757,45 +1775,54 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
struct packet_type *ptype;
struct sk_buff *skb2 = NULL;
struct packet_type *pt_prev = NULL;
+ struct list_head *ptype_list = &ptype_all;
rcu_read_lock();
- list_for_each_entry_rcu(ptype, &ptype_all, list) {
+again:
+ list_for_each_entry_rcu(ptype, ptype_list, list) {
/* Never send packets back to the socket
* they originated from - MvS (miquels@drinkel.ow.org)
*/
- if ((ptype->dev == dev || !ptype->dev) &&
- (!skb_loop_sk(ptype, skb))) {
- if (pt_prev) {
- deliver_skb(skb2, pt_prev, skb->dev);
- pt_prev = ptype;
- continue;
- }
+ if (skb_loop_sk(ptype, skb))
+ continue;
- skb2 = skb_clone(skb, GFP_ATOMIC);
- if (!skb2)
- break;
+ if (pt_prev) {
+ deliver_skb(skb2, pt_prev, skb->dev);
+ pt_prev = ptype;
+ continue;
+ }
- net_timestamp_set(skb2);
+ /* need to clone skb, done only once */
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (!skb2)
+ goto out_unlock;
- /* skb->nh should be correctly
- set by sender, so that the second statement is
- just protection against buggy protocols.
- */
- skb_reset_mac_header(skb2);
-
- if (skb_network_header(skb2) < skb2->data ||
- skb_network_header(skb2) > skb_tail_pointer(skb2)) {
- net_crit_ratelimited("protocol %04x is buggy, dev %s\n",
- ntohs(skb2->protocol),
- dev->name);
- skb_reset_network_header(skb2);
- }
+ net_timestamp_set(skb2);
- skb2->transport_header = skb2->network_header;
- skb2->pkt_type = PACKET_OUTGOING;
- pt_prev = ptype;
+ /* skb->nh should be correctly
+ * set by sender, so that the second statement is
+ * just protection against buggy protocols.
+ */
+ skb_reset_mac_header(skb2);
+
+ if (skb_network_header(skb2) < skb2->data ||
+ skb_network_header(skb2) > skb_tail_pointer(skb2)) {
+ net_crit_ratelimited("protocol %04x is buggy, dev %s\n",
+ ntohs(skb2->protocol),
+ dev->name);
+ skb_reset_network_header(skb2);
}
+
+ skb2->transport_header = skb2->network_header;
+ skb2->pkt_type = PACKET_OUTGOING;
+ pt_prev = ptype;
+ }
+
+ if (ptype_list == &ptype_all) {
+ ptype_list = &dev->ptype_all;
+ goto again;
}
+out_unlock:
if (pt_prev)
pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
rcu_read_unlock();
@@ -2617,7 +2644,7 @@ static int xmit_one(struct sk_buff *skb, struct net_device *dev,
unsigned int len;
int rc;
- if (!list_empty(&ptype_all))
+ if (!list_empty(&ptype_all) || !list_empty(&dev->ptype_all))
dev_queue_xmit_nit(skb, dev);
len = skb->len;
@@ -3615,7 +3642,6 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
struct packet_type *ptype, *pt_prev;
rx_handler_func_t *rx_handler;
struct net_device *orig_dev;
- struct net_device *null_or_dev;
bool deliver_exact = false;
int ret = NET_RX_DROP;
__be16 type;
@@ -3658,11 +3684,15 @@ another_round:
goto skip_taps;
list_for_each_entry_rcu(ptype, &ptype_all, list) {
- if (!ptype->dev || ptype->dev == skb->dev) {
- if (pt_prev)
- ret = deliver_skb(skb, pt_prev, orig_dev);
- pt_prev = ptype;
- }
+ if (pt_prev)
+ ret = deliver_skb(skb, pt_prev, orig_dev);
+ pt_prev = ptype;
+ }
+
+ list_for_each_entry_rcu(ptype, &skb->dev->ptype_all, list) {
+ if (pt_prev)
+ ret = deliver_skb(skb, pt_prev, orig_dev);
+ pt_prev = ptype;
}
skip_taps:
@@ -3718,19 +3748,21 @@ ncls:
skb->vlan_tci = 0;
}
+ type = skb->protocol;
+
/* deliver only exact match when indicated */
- null_or_dev = deliver_exact ? skb->dev : NULL;
+ if (likely(!deliver_exact)) {
+ deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
+ &ptype_base[ntohs(type) &
+ PTYPE_HASH_MASK]);
+ }
- type = skb->protocol;
- list_for_each_entry_rcu(ptype,
- &ptype_base[ntohs(type) & PTYPE_HASH_MASK], list) {
- if (ptype->type == type &&
- (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
- ptype->dev == orig_dev)) {
- if (pt_prev)
- ret = deliver_skb(skb, pt_prev, orig_dev);
- pt_prev = ptype;
- }
+ deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
+ &orig_dev->ptype_specific);
+
+ if (unlikely(skb->dev != orig_dev)) {
+ deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
+ &skb->dev->ptype_specific);
}
if (pt_prev) {
@@ -6579,6 +6611,8 @@ void netdev_run_todo(void)
/* paranoia */
BUG_ON(netdev_refcnt_read(dev));
+ BUG_ON(!list_empty(&dev->ptype_all));
+ BUG_ON(!list_empty(&dev->ptype_specific));
WARN_ON(rcu_access_pointer(dev->ip_ptr));
WARN_ON(rcu_access_pointer(dev->ip6_ptr));
WARN_ON(dev->dn_ptr);
@@ -6761,6 +6795,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
INIT_LIST_HEAD(&dev->adj_list.lower);
INIT_LIST_HEAD(&dev->all_adj_list.upper);
INIT_LIST_HEAD(&dev->all_adj_list.lower);
+ INIT_LIST_HEAD(&dev->ptype_all);
+ INIT_LIST_HEAD(&dev->ptype_specific);
dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
setup(dev);
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCHv3 ipsec-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host
From: David Miller @ 2015-01-27 19:24 UTC (permalink / raw)
To: David.Laight; +Cc: fan.du, steffen.klassert, herbert, netdev, fengyuleidian0615
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD3B2B@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 27 Jan 2015 09:46:21 +0000
> From: Fan Du
>> structure like xfrm_usersa_info or xfrm_userpolicy_info
>> has different sizeof when compiled as 32bits and 64bits
>> due to not appending pack attribute in their definition.
>
> Don't 'pack' the structure, just ensure that all the fields
> are fixed sized and on their natural boundary.
This horse went out of the door more than a decade ago, we can't
change the layout of any of these structures and must at some point
add code to translate instead.
^ permalink raw reply
* Does arping update arp table?
From: Murali Karicheri @ 2015-01-27 19:23 UTC (permalink / raw)
To: netdev; +Cc: David Miller
Experts,
The ARP code (net/ipv4/arp.c) seems to treat arp response resulting from
a arping as an unsolicited response and doesn't update the arp table.
This get updated if the per interface accept sys ctrl is enabled. This
command seems to have been updating the arp table in older kernel. For
example on a Ubuntu 12.04 based kernel I see
a0868495@ula0868495 ~/Project/linux-keystone $ sudo arping 192.168.1.10
-c 1 -f
ARPING 192.168.1.10 from 192.168.1.9 eth0
Unicast reply from 192.168.1.10 [00:1B:A9:02:26:10] 5.411ms
Sent 1 probes (1 broadcast(s))
Received 1 response(s)
a0868495@ula0868495 ~/Project/linux-keystone $ arp -n -a
? (192.168.1.10) at 00:1b:a9:02:26:10 [ether] on eth0
? (192.168.1.1) at 00:1f:90:6d:5c:a8 [ether] on eth0
a0868495@ula0868495 ~/Project/linux-keystone $ uname -a
Linux ula0868495 3.2.0-23-generic #36-Ubuntu SMP Tue Apr 10 20:41:14 UTC
2012 i686 i686 i386 GNU/Linux
However this command doesn't update the arp table on my Keystone EVM
based on v3.19.x and also newer ubuntu machine based on v3.11. Is this
expected behavior? I believe the arp response resulting from a arping
command is controlled through per interface accept sys control like as
in gratuitous arp. Please repond.
Thanks and regards,
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net: phy: prevent double suspend
From: Florian Fainelli @ 2015-01-27 19:16 UTC (permalink / raw)
To: Sergei Shtylyov, netdev; +Cc: davem, s.hauer, b38611
In-Reply-To: <54C7E140.4080908@cogentembedded.com>
On 27/01/15 11:04, Sergei Shtylyov wrote:
> Hello.
>
> On 01/27/2015 09:05 AM, Florian Fainelli wrote:
>
>> This patch series addresses a problem that Fugang and I observed on
>> different
>> platforms where a given PHY device might end-up being suspended twice.
>
>> Once as part of the call from ndo_open() all the way down to
>> phy_detach() and
>> phy_suspend() and a second time when the generic platform device/driver
>> suspend/resume callbacks are called in drivers/net/phy/mdio_bus.c.
>
>> Thanks to Fugang for giving this a quick try on i.MX6/FEC and reporting
>> positive test results!
>
> I can now confirm this patchset fixed our issue with an external
> abort during suspend on R-Car platform (with the 'sh_eth' driver)!
Great, thanks Sergei!
>
>> Florian Fainelli (4):
>> net: phy: utilize phy_suspend and phy_resume
>> net: phy: document has_fixups field
>> net: phy: keep track of the PHY suspend state
>> net: phy: avoid suspending twice a PHY
>
> WBR, Sergei
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit
From: David Miller @ 2015-01-27 19:13 UTC (permalink / raw)
To: David.Laight; +Cc: amirv, netdev, ogerlitz, yevgenyp, jackm
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD3B01@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 27 Jan 2015 09:43:27 +0000
>> __be64 out_param;
>> __be16 token;
>> u16 reserved;
>> u8 status;
>> u8 flags;
>> __be16 opcode;
>> -};
>> +} __packed;
>
> Don't add '__packed' unless you expect the structure to be misaligned
> in memory.
> On systems that fault mis-aligned memory requests you've requested the
> compiler generate code to read/write everything using byte sized memory
> accesses and a lot of shifting and masking.
Indeed, I'm really sick of seeing these packed structures being
created all over the place.
They are to be used in absolutely extreme cases where no other
solution is possible.
^ permalink raw reply
* Re: [PATCH 0/2] sh_eth: massage PM code
From: Sergei Shtylyov @ 2015-01-27 19:06 UTC (permalink / raw)
To: netdev, David Miller; +Cc: linux-sh
In-Reply-To: <5608250.lrFkJTjgSf@wasted.cogentembedded.com>
Hello.
On 01/22/2015 01:16 AM, Sergei Shtylyov wrote:
> Here's a set of 2 patches against DaveM's 'net-next.git' repo. We're adding
> the support for suspend/hibernation as well as somewhat changing the existing
> code. There are still MDIO-related issue with suspend (kernel exception), we've
> been working on it and shall address it with a separate patch...
It turned out that Florian Fainelli's recent series was the best approach
to fixing this issue. We were only looking at our driver while it was a common
MDIO problem...
> [1/2] sh_eth: use SET_RUNTIME_PM_OPS()
> [2/2] sh_eth: add more PM methods
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net: phy: prevent double suspend
From: Sergei Shtylyov @ 2015-01-27 19:04 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, s.hauer, b38611
In-Reply-To: <1422338740-18418-1-git-send-email-f.fainelli@gmail.com>
Hello.
On 01/27/2015 09:05 AM, Florian Fainelli wrote:
> This patch series addresses a problem that Fugang and I observed on different
> platforms where a given PHY device might end-up being suspended twice.
> Once as part of the call from ndo_open() all the way down to phy_detach() and
> phy_suspend() and a second time when the generic platform device/driver
> suspend/resume callbacks are called in drivers/net/phy/mdio_bus.c.
> Thanks to Fugang for giving this a quick try on i.MX6/FEC and reporting
> positive test results!
I can now confirm this patchset fixed our issue with an external abort
during suspend on R-Car platform (with the 'sh_eth' driver)!
> Florian Fainelli (4):
> net: phy: utilize phy_suspend and phy_resume
> net: phy: document has_fixups field
> net: phy: keep track of the PHY suspend state
> net: phy: avoid suspending twice a PHY
WBR, Sergei
^ permalink raw reply
* RE: Fw: [Bug 92081] New: skb->len=0 and getting "EOF on netlink" with "ip monitor all" (of iproute) when adding a vlan with "bridge vlan add"
From: Rosen, Rami @ 2015-01-27 18:38 UTC (permalink / raw)
To: roopa, Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <54C7BEA4.9000406@cumulusnetworks.com>
Hi, Roopa,
> I think my below commit fixed one case of such error:
I am well aware of your commit (in fact I even sent cleanup patch on top of it, removing the oflags, which was applied).
It seems to me that this commit of yours does not avoid the specific problem of getting EOF with "ip monitor all" which is described in the BUG I opened; it
could be that it avoid problem with other scenarios, and with wrong message size when both SELF and MASTER flags are set.
> The reason for the zero length message in this case is that the user is sending
> the setlink request to the bridge with self flag set.
> And since the getlink on the bridge device only returns bytes when its a bridge port, there are no bytes in the skb.
> I will reconfirm that the above is true and submit a patch (I can update the bugzilla link below as well).
This is exactly so, I am fully confident about it, I checked it in depth with debug , and I had printed the skb->len before calling rtnl_notify() in
rtnl_bridge_notify() in net/core/rtnetlink.c under such scenario described in the BUG mentioned in the bugzilla link and it was indeed 0.
For the sake of those who are interested in more implementation details and in the code walkthrough under such scenario, what happens when "bridge vlan add vid 1 dev br0 self" , you should follow this path:
Look at rtnl_bridge_setlink() method, it is invoked in this case.
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2782
If the SELF flag is set it calls dev->netdev_ops->ndo_bridge_setlink()
See:
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2840
and then it calls rtnl_bridge_notify()
See:
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2850
Now, rtnl_bridge_notify() calls dev->netdev_ops->ndo_bridge_getlink()
when the self flag is set.
See:
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2767
Now, when running the "bridge vlan add" on a bridge device like we do (and **not on a bridge port**)
then the dev variable is an instance of a software bridge. So this calls the ndo_bridge_getlink() callback of the software bridge, which is br_getlink():
See:
http://lxr.free-electrons.com/source/net/bridge/br_netlink.c#L205
Now, br_getlink() first checks if the device is a bridge port:
struct net_bridge_port *port = br_port_get_rtnl(dev);
And it returns 0 if not.
So as a result, the skb->len is 0 and an empty notification is sent.
And when the rtneltnlink socket, which is opened by "ip monitor all" and listens to netlink messages, receives an
empty notification it terminates with the "EOF" message (as mentioned in the bugzilla link).
Sending a patch for resolving it and updating the bugzilla will be really great!
Regards,
Rami Rosen
Intel Corporation
-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of roopa
Sent: Tuesday, January 27, 2015 18:37
To: Stephen Hemminger
Cc: netdev@vger.kernel.org
Subject: Re: Fw: [Bug 92081] New: skb->len=0 and getting "EOF on netlink" with "ip monitor all" (of iproute) when adding a vlan with "bridge vlan add"
I noticed this during my recent cleanup of rtnl_bridge_setlink/rtnl_bridge_dellink.
I think my below commit fixed one case of such error:
commit 02dba4388d1691a087f40fe8acd2e1ffd577a07f
Author: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Wed Jan 14 20:02:25 2015 -0800
bridge: fix setlink/dellink notifications
The reason for the zero length message in this case is that the user is sending
the setlink request to the bridge with self flag set.
And since the getlink on the bridge device only returns bytes when its a bridge port, there are no bytes in the skb.
I will reconfirm that the above is true and submit a patch (I can update the bugzilla link below as well).
Thanks,
Roopa
On 1/27/15, 4:01 AM, Stephen Hemminger wrote:
>
> Begin forwarded message:
>
> Date: Mon, 26 Jan 2015 10:15:12 -0800
> From: "bugzilla-daemon@bugzilla.kernel.org"
> <bugzilla-daemon@bugzilla.kernel.org>
> To: "stephen@networkplumber.org" <stephen@networkplumber.org>
> Subject: [Bug 92081] New: skb->len=0 and getting "EOF on netlink" with "ip monitor all" (of iproute) when adding a vlan with "bridge vlan add"
>
>
> https://bugzilla.kernel.org/show_bug.cgi?id=92081
>
> Bug ID: 92081
> Summary: skb->len=0 and getting "EOF on netlink" with "ip
> monitor all" (of iproute) when adding a vlan with
> "bridge vlan add"
> Product: Networking
> Version: 2.5
> Kernel Version: 3.17.6-300
> Hardware: All
> OS: Linux
> Tree: Fedora
> Status: NEW
> Severity: high
> Priority: P1
> Component: Other
> Assignee: shemminger@linux-foundation.org
> Reporter: ramirose@gmail.com
> Regression: No
>
> On Fedora 21, with 3.17.6-300.fc21.x86_64, with iproute-3.16.0-3
> (installed from rpm), ip -V:
> ip utility, iproute2-ss140804
>
> Running in one terminal:
> ip monitor all
>
> And then running in a second terminal this sequence:
> ip link add br0 type bridge
> bridge vlan add vid 10 dev br0 self
>
> causes the "ip monitor all" to terminate, with "EOF on netlink".
>
> This happens also on older distros of Fedora (Fedora 20 and downward)
> with older kernels.
>
> It seems that the reason is that an skb->len is 0 for the netlink
> notification which is sent from with rtnl_notify() which is invoked
> from rtnl_bridge_notify(), which in turn is invoked from
> rtnl_bridge_setlink().
>
> See:
> http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2773
>
> Rami Rosen
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" 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
* Re: [PATCH 3/3] net: allwinner: sun4i-emac: fix emac SRAM mapping
From: Maxime Ripard @ 2015-01-27 18:13 UTC (permalink / raw)
To: Jens Kuske
Cc: Arnd Bergmann, Lee Jones, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chen-Yu Tsai,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <54C65089.70305-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5025 bytes --]
Hi,
On Mon, Jan 26, 2015 at 03:34:49PM +0100, Jens Kuske wrote:
> Hi,
>
> On 25/01/15 17:25, Maxime Ripard wrote:
> >Hi Jens,
> >
> >On Sun, Jan 25, 2015 at 04:49:19PM +0100, Jens Kuske wrote:
> >>The EMAC needs SRAM block A3_A4 being mapped to EMAC peripheral to
> >>work. This is done by the bootloader most of the time, but U-Boot
> >>Falcon Mode, for example, skips emac initialization and SRAM would
> >>stay mapped to the CPU.
> >
> >Thanks for reviving this.
> >
> >>Signed-off-by: Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>---
> >> drivers/net/ethernet/allwinner/Kconfig | 1 +
> >> drivers/net/ethernet/allwinner/sun4i-emac.c | 18 ++++++++++++++++++
> >> 2 files changed, 19 insertions(+)
> >>
> >>diff --git a/drivers/net/ethernet/allwinner/Kconfig b/drivers/net/ethernet/allwinner/Kconfig
> >>index d8d95d4..508a288 100644
> >>--- a/drivers/net/ethernet/allwinner/Kconfig
> >>+++ b/drivers/net/ethernet/allwinner/Kconfig
> >>@@ -28,6 +28,7 @@ config SUN4I_EMAC
> >> select MII
> >> select PHYLIB
> >> select MDIO_SUN4I
> >>+ select MFD_SYSCON
> >> ---help---
> >> Support for Allwinner A10 EMAC ethernet driver.
> >>
> >>diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
> >>index 1fcd556..86c891d 100644
> >>--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
> >>+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
> >>@@ -18,6 +18,8 @@
> >> #include <linux/gpio.h>
> >> #include <linux/interrupt.h>
> >> #include <linux/irq.h>
> >>+#include <linux/mfd/syscon.h>
> >>+#include <linux/mfd/syscon/sun4i-sc.h>
> >> #include <linux/mii.h>
> >> #include <linux/module.h>
> >> #include <linux/netdevice.h>
> >>@@ -28,6 +30,7 @@
> >> #include <linux/of_platform.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/phy.h>
> >>+#include <linux/regmap.h>
> >>
> >> #include "sun4i-emac.h"
> >>
> >>@@ -78,6 +81,7 @@ struct emac_board_info {
> >>
> >> struct phy_device *phy_dev;
> >> struct device_node *phy_node;
> >>+ struct regmap *sc;
> >> unsigned int link;
> >> unsigned int speed;
> >> unsigned int duplex;
> >>@@ -862,6 +866,18 @@ static int emac_probe(struct platform_device *pdev)
> >> goto out;
> >> }
> >>
> >>+ /* Map SRAM_A3_A4 to EMAC */
> >>+ db->sc = syscon_regmap_lookup_by_compatible(
> >>+ "allwinner,sun4i-a10-syscon");
> >>+ if (IS_ERR(db->sc)) {
> >>+ dev_err(&pdev->dev, "failed to find syscon regmap\n");
> >>+ ret = PTR_ERR(db->sc);
> >>+ goto out;
> >>+ }
> >>+
> >>+ regmap_update_bits(db->sc, SUN4I_SC1, SUN4I_SC1_SRAM_A3_A4_MAP_MASK,
> >>+ SUN4I_SC1_SRAM_A3_A4_MAP_EMAC);
> >>+
> >
> >I don't think that using a syscon is the right solution here.
> >
> >All this SRAM mapping thing is mutually exclusive, and will possibly
> >impact other drivers as well.
>
> Each single SRAM area can only be mapped to a single peripheral, so as long
> as the driver only changes bits related to his own area nothing can go wrong
> I believe.
Which is exactly my point. This kind of assumption is very
fragile. Such mistakes might be made, will slip in under any review
and might get un-noticed for a very long time.
While adding a simple driver at least would make this obvious when two
drivers will contend for the same SRAM mapping.
> SRAM_C2 looks like it can be mapped do different devices (AE, CE, ACE), but
> as far as I understand this, they are all related to the ACE device, sharing
> a common register space, and would have to be handled by a single driver
> anyway (if that will ever happen without docs)
> https://linux-sunxi.org/ACE_Register_guide
>
> >I think this is a more a case for a small driver in drivers/soc that
> >would take care of this, and make sure that client drivers don't step
> >on each other's toe.
>
> I'm not convinced this is necessary, but what would this driver do different
> than a basic regmap? Check if the area is already mapped by any driver and
> deny mapping it again by a different driver? Which different driver, each
> area is only interesting for a single device/driver? Except maybe mapping it
> to CPU as general purpose sram, but that would need some direct agreement
> with the driver to steal its memory anyway.
Off the top of my head:
- Make sure no one step on each other's toe, including the CPU that
might require a SRAM for several things (suspend, cpuidle, PSCI,
etc.)
- Provide a comprehensive status of the various SRAM and what they
are mapped to in debugfs
- Provide an abstraction to the SRAM mapping IP. You make the
assumption that the client drivers will always use on an A10 or an
SoC that has the same SRAMs, and the same IP to control which
device they are mapped to. This might not be true for other
drivers, and other SoCs.
- Make any adjustment to these registers that wouldn't fit in any
client drivers.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ 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