* Re: [PATCH] net: less interrupt masking in NAPI
From: Yang Yingliang @ 2014-12-03 9:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, willemb
In-Reply-To: <1417592482.5303.132.camel@edumazet-glaptop2.roam.corp.google.com>
On 2014/12/3 15:41, Eric Dumazet wrote:
> On Wed, 2014-12-03 at 15:31 +0800, Yang Yingliang wrote:
>
>> This patch can resolve my performance problem.
>> Will/Can this patch queue for stable ?
>
> Hmm.. please give us more details, I am surprised it can have
> a big impact.
>
>
>
>
Before this patch, when a large network flow arrives, some other processes
response slowly or even don't response because the cpu is dealing with softirq.
After this patch, under pressure, much more softirq is doing in ksoftirqd. The other
processes be scheduled.
My system has dual core.
Regards,
Yang
^ permalink raw reply
* RE: [PATCH net-next] r8152: reduce memory copy for rx
From: Hayes Wang @ 2014-12-03 9:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
In-Reply-To: <1417590925.5303.127.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Wednesday, December 03, 2014 3:15 PM
[...]
> Have you tried using more concurrent RX flows, in a possibly lossy
> environment (so that TCP is forced to queue packets in out of order
> queue) ?
I don't do the test. I would check it next time.
> skb cloning prevents GRO and TCP coalescing from working.
>
> netfilter might also be forced to copy whole frame in case a mangle is
> needed (eg with NAT ...)
>
> I would rather try to implement GRO, and/or using fragments instead of
> pure linear skbs.
>
> (skb->head would be around 128 or 256 bytes, and you attach to skb the
> frame as a page fragment)
Thanks for your response. I would study the GRO first.
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Michael S. Tsirkin @ 2014-12-03 9:03 UTC (permalink / raw)
To: Jesse Gross
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Wang,
Du, Fan, fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <CAEP_g=-86Z6pxNow-wjnbx_v9er_TSn6x5waigqVqYHa7tEQJw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Dec 02, 2014 at 10:12:04AM -0800, Jesse Gross wrote:
> On Tue, Dec 2, 2014 at 9:41 AM, Thomas Graf <tgraf@suug.ch> wrote:
> > On 12/02/14 at 07:34pm, Michael S. Tsirkin wrote:
> >> On Tue, Dec 02, 2014 at 05:09:27PM +0000, Thomas Graf wrote:
> >> > On 12/02/14 at 01:48pm, Flavio Leitner wrote:
> >> > > What about containers or any other virtualization environment that
> >> > > doesn't use Virtio?
> >> >
> >> > The host can dictate the MTU in that case for both veth or OVS
> >> > internal which would be primary container plumbing techniques.
> >>
> >> It typically can't do this easily for VMs with emulated devices:
> >> real ethernet uses a fixed MTU.
> >>
> >> IMHO it's confusing to suggest MTU as a fix for this bug, it's
> >> an unrelated optimization.
> >> ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED is the right fix here.
> >
> > PMTU discovery only resolves the issue if an actual IP stack is
> > running inside the VM. This may not be the case at all.
>
> It's also only really a correct thing to do if the ICMP packet is
> coming from an L3 node. If you are doing straight bridging then you
> have to resort to hacks like OVS had before, which I agree are not
> particularly desirable.
The issue seems to be that fundamentally, this is
bridging interfaces with variable MTUs (even if MTU values
on devices don't let us figure this out)-
that is already not straight bridging, and
I would argue sending ICMPs back is the right thing to do.
> > I agree that exposing an MTU towards the guest is not applicable
> > in all situations, in particular because it is difficult to decide
> > what MTU to expose. It is a relatively elegant solution in a lot
> > of virtualization host cases hooked up to an orchestration system
> > though.
>
> I also think this is the right thing to do as a common case
> optimization and I know other platforms (such as Hyper-V) do it. It's
> not a complete solution so we still need the original patch in this
> thread to handle things transparently.
Well, as I believe David (and independently Jason) is saying, it looks like
the ICMPs we are sending back after applying the original patch have the
wrong MTU.
And if I understand what David is saying here, IP is also the wrong place to
do it.
--
MST
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH] bpf: arm64: lift restriction on last instruction
From: Zi Shen Lim @ 2014-12-03 8:38 UTC (permalink / raw)
To: Alexei Starovoitov, David S. Miller, Catalin Marinas, Will Deacon
Cc: Zi Shen Lim, Daniel Borkmann, netdev, linux-arm-kernel,
linux-kernel
Earlier implementation assumed last instruction is BPF_EXIT.
Since this is no longer a restriction in eBPF, we remove this
limitation.
Per Alexei Starovoitov [1]:
> classic BPF has a restriction that last insn is always BPF_RET.
> eBPF doesn't have BPF_RET instruction and this restriction.
> It has BPF_EXIT insn which can appear anywhere in the program
> one or more times and it doesn't have to be last insn.
[1] https://lkml.org/lkml/2014/11/27/2
Fixes: e54bcde3d69d ("arm64: eBPF JIT compiler")
Signed-off-by: Zi Shen Lim <zlim.lnx@gmail.com>
---
arch/arm64/net/bpf_jit_comp.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 41f1e3e..edba042 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -60,7 +60,7 @@ struct jit_ctx {
const struct bpf_prog *prog;
int idx;
int tmp_used;
- int body_offset;
+ int epilogue_offset;
int *offset;
u32 *image;
};
@@ -130,8 +130,8 @@ static void jit_fill_hole(void *area, unsigned int size)
static inline int epilogue_offset(const struct jit_ctx *ctx)
{
- int to = ctx->offset[ctx->prog->len - 1];
- int from = ctx->idx - ctx->body_offset;
+ int to = ctx->epilogue_offset;
+ int from = ctx->idx;
return to - from;
}
@@ -463,6 +463,8 @@ emit_cond_jmp:
}
/* function return */
case BPF_JMP | BPF_EXIT:
+ /* Optimization: when last instruction is EXIT,
+ simply fallthrough to epilogue. */
if (i == ctx->prog->len - 1)
break;
jmp_offset = epilogue_offset(ctx);
@@ -685,11 +687,13 @@ void bpf_int_jit_compile(struct bpf_prog *prog)
/* 1. Initial fake pass to compute ctx->idx. */
- /* Fake pass to fill in ctx->offset. */
+ /* Fake pass to fill in ctx->offset and ctx->tmp_used. */
if (build_body(&ctx))
goto out;
build_prologue(&ctx);
+
+ ctx.epilogue_offset = ctx.idx;
build_epilogue(&ctx);
/* Now we know the actual image size. */
@@ -706,7 +710,6 @@ void bpf_int_jit_compile(struct bpf_prog *prog)
build_prologue(&ctx);
- ctx.body_offset = ctx.idx;
if (build_body(&ctx)) {
bpf_jit_binary_free(header);
goto out;
--
1.9.1
^ permalink raw reply related
* Re: linux-next: manual merge of the driver-core tree with the net-next tree
From: Jeremiah Mahler @ 2014-12-03 8:36 UTC (permalink / raw)
To: Arend van Spriel
Cc: Stephen Rothwell, Greg KH, John W. Linville, David Miller, netdev,
linux-next, linux-kernel, Felix Fietkau, Ben Greear
In-Reply-To: <547C19ED.10201@broadcom.com>
all,
On Mon, Dec 01, 2014 at 08:34:05AM +0100, Arend van Spriel wrote:
> On 01-12-14 08:19, Stephen Rothwell wrote:
> > Hi Greg,
> >
> > Today's linux-next merge of the driver-core tree got a conflict in
> > drivers/net/wireless/ath/ath9k/debug.c between commits 70e535ed0029
> > ("ath9k: clean up debugfs print of reset causes"), 7b8aaead958e
> > ("ath9k: restart hardware after noise floor calibration failure") and
> > 325e18817668 ("ath9k: fix misc debugfs when not using chan context")
> > from the net-next tree and commit 631bee257bd5 ("ath: use seq_file api
> > for ath9k debugfs files") from the driver-core tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary (no action
> > is required).
> >
> > Greg, I am not sure why those 2 commits are even in your tree. Do they
> > depend on something else in your tree?
>
> They do. The three commits below are related:
>
> d32394f ath: ath9k: use debugfs_create_devm_seqfile() helper for
> seq_file entrie
> 631bee2 ath: use seq_file api for ath9k debugfs files
> 98210b7 debugfs: add helper function to create device related seq_file
>
> The ath patches were made to provide example of using the new helper
> function and get some idea about code savings. Greg and John discussed
> who would take them. I noticed other ath changes in net-next so I kinda
> expected this email ;-)
>
> Regards,
> Arend
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
I just ran in to a problem with one of these commits.
On an Acer C720 laptop if a suspend is performed the screen freezes,
the machine locks up, and according to the indicator lights it does
not enter suspend. A hard reset is required to get it running again.
I have bisected the kernel and found that the following is the first bad
commit.
commit d32394fae95741d733b174ec1446f27765f80233
Author: Arend van Spriel <arend@broadcom.com>
Date: Sun Nov 9 11:32:00 2014 +0100
ath: ath9k: use debugfs_create_devm_seqfile() helper for seq_file
entries
Use the helper to get rid of the file operations per debugfs file.
The
struct ath9k_softc pointer is set as device driver data to be
obtained
in the seq_file read operation.
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Let me know if I can do anything else to help.
--
- Jeremiah Mahler
^ permalink raw reply
* wireless workshop - was: Netdev 0.1 Call for Proposals
From: Johannes Berg @ 2014-12-03 8:24 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: netdev, linux-wireless, lwn.net, netdev01
In-Reply-To: <20141203022815.GM4612@toccata2.tricolour.ca>
On Tue, 2014-12-02 at 21:28 -0500, Richard Guy Briggs wrote:
> 1) Workshops (day 1)
FWIW, we're planning a wireless summit as a workshop here, which should
give us a little more breathing room than e.g. the 3-hour slot at LPC.
I'll put up the wiki page and other things and start working on an
agenda when I'm back home next week.
Wireless-related submissions for the remainder of the conference are
also very welcome and encouraged!
johannes
^ permalink raw reply
* What's the concern about setting irq thread's policy as SCHED_FIFO
From: Qin Chuanyu @ 2014-12-03 8:06 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
I am doing network performance test under suse11sp3 and intel 82599 nic,
Becasuse the softirq is out of schedule policy's control, so netserver
thread couldn't always get 100% cpu usage, then packet dropped in kernel
udp socket's receive queue.
In order to get a stable result, I did some patch in ixgbe driver and
then use irq_thread instead of softirq to handle rx.
It seems work well, but irq_thread's SCHED_FIFO schedule policy cause
that when the cpu is limited, netserver couldn't work at all.
So I change the irq_thread's schedule policy from SCHED_FIFO to
SCHED_NORMAL, then the irq_thread could share the cpu usage with
netserver thread.
the question is:
What's the concrete reason about setting irq thread's policy as SCHED_FIFO?
Except the priority affecting the cpu usage, any function would be
broken if irq thread change to SCHED_NORMAL?
^ permalink raw reply
* Re: [PATCH net] cxgb4: Add a check for flashing FW using ethtool
From: Hariprasad S @ 2014-12-03 8:07 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, davem, leedom, anish, nirranjan, kumaras
In-Reply-To: <547DCA45.7060702@cogentembedded.com>
On Tue, Dec 02, 2014 at 17:18:45 +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 12/2/2014 3:09 PM, Hariprasad Shenai wrote:
>
> >Don't let T4 firmware flash on a T5 adapter and vice-versa
> >using ethtool
>
> >Based on original work by Casey Leedom <leedom@chelsio.com>
>
> >Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
> >---
> > drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 26 ++++++++++++++++++++++++++
> > 1 files changed, 26 insertions(+), 0 deletions(-)
>
> >diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> >index 163a2a1..fae205a 100644
> >--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> >+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> >@@ -1131,6 +1131,27 @@ unsigned int t4_flash_cfg_addr(struct adapter *adapter)
> > return FLASH_CFG_START;
> > }
> >
> >+/* Return TRUE if the specified firmware matches the adapter. I.e. T4
> >+ * firmware for T4 adapters, T5 firmware for T5 adapters, etc. We go ahead
> >+ * and emit an error message for mismatched firmware to save our caller the
> >+ * effort ...
> >+ */
> >+static int t4_fw_matches_chip(const struct adapter *adap,
>
> s/int/bool/?
>
> >+ const struct fw_hdr *hdr)
> >+{
> >+ /* The expression below will return FALSE for any unsupported adapter
> >+ * which will keep us "honest" in the future ...
> >+ */
> >+ if ((is_t4(adap->params.chip) && hdr->chip == FW_HDR_CHIP_T4) ||
> >+ (is_t5(adap->params.chip) && hdr->chip == FW_HDR_CHIP_T5))
> >+ return 1;
>
> s/1/true/?
>
> >+
> >+ dev_err(adap->pdev_dev,
> >+ "FW image (%d) is not suitable for this adapter (%d)\n",
> >+ hdr->chip, CHELSIO_CHIP_VERSION(adap->params.chip));
> >+ return 0;
>
> s/0/false/?
>
> [...]
>
> WBR, Sergei
>
Thanks for the review comment, have sent a V2 based on your comments.
^ permalink raw reply
* Re: [PATCH v2 1/6] GMAC: add driver for Rockchip RK3288 SoCs integrated GMAC
From: Roger @ 2014-12-03 7:57 UTC (permalink / raw)
To: Heiko Stübner
Cc: peppe.cavallaro, netdev, linux-kernel, linux-rockchip, kever.yang,
eddie.cai
In-Reply-To: <1591535.72TyRu5yfQ@diego>
Hi! Heiko
about regulator, power gpio, reset gpio and irq gpio
please refer to my comment inline, tks.
On 2014/12/2 7:44, Heiko Stübner wrote:
> Hi Roger,
>
> the comments inline are a rough first review. I hope to get a clearer picture
> for the stuff I'm not sure about in v3 once the big issues are fixed.
>
>
> Am Donnerstag, 27. November 2014, 10:52:08 schrieb Roger Chen:
>> This driver is based on stmmac driver.
>>
>> modification based on Giuseppe CAVALLARO's suggestion:
>> 1. use BIT()
>>
>> > +/*RK3288_GRF_SOC_CON3*/
>> > +#define GMAC_TXCLK_DLY_ENABLE ((0x4000 << 16) | (0x4000))
>> > +#define GMAC_TXCLK_DLY_DISABLE ((0x4000 << 16) | (0x0000))
>>
>> ...
>>
>> why do not use BIT and BIT_MASK where possible?
>>
>> ===>after modification:
>>
>> #define GRF_BIT(nr) (BIT(nr) | BIT(nr+16))
>> #define GRF_CLR_BIT(nr) (BIT(nr+16))
>> #define GMAC_TXCLK_DLY_ENABLE GRF_BIT(14)
>> #define GMAC_TXCLK_DLY_DISABLE GRF_CLR_BIT(14)
>> ...
>> 2.
>>
>> > + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> > + GMAC_PHY_INTF_SEL_RGMII);
>> > + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> > + GMAC_RMII_MODE_CLR);
>>
>> maybe you could perform just one write unless there is some HW
>> constraint.
>>
>> ===>after modification:
>>
>> regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> GMAC_PHY_INTF_SEL_RGMII | GMAC_RMII_MODE_CLR);
>>
>> 3. use macros
>>
>> > + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO3D_E, 0xFFFFFFFF);
>> > + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4B_E,
>> > + 0x3<<2<<16 | 0x3<<2);
>>
>> pls use macros, these shift sequence is really help to decode
>>
>> ===>after modification:
>>
>> regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4A_E, GPIO4A_12MA);
>> regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4B_E, GPIO4B_2_12MA);
>>
>> 4. remove grf fail check in rk_gmac_setup()
>>
>> > + if (IS_ERR(bsp_priv->grf))
>> > + dev_err(&pdev->dev, "Missing rockchip,grf property\n");
>>
>> I wonder if you can fail on here and save all the check in
>> set_rgmii_speed etc.
>> Maybe this can be considered a mandatory property for the glue-logic.
>>
>> 5. remove .tx_coe=1
>>
>> > +const struct stmmac_of_data rk_gmac_data = {
>> > + .has_gmac = 1,
>> > + .tx_coe = 1,
>>
>> FYI, on new gmac there is the HW capability register to dinamically
>> provide you if coe is supported.
>>
>> IMO you should add the OF "compatible" string and in case of mac
>> newer than the 3.50a you can remove coe.
> changelogs like these, should be compact and also not be in the commit message
> itself, but in the "comment"-section below the "---" and before the diffstat.
>
>
>> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
>> ---
> changelog here ... the commonly used pattern is something like
>
> changes since v2:
> - ...
> - ...
>
> changes since v1:
> - ...
>
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
>> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 636
>> ++++++++++++++++++++ .../net/ethernet/stmicro/stmmac/stmmac_platform.c |
>> 3 +
>> .../net/ethernet/stmicro/stmmac/stmmac_platform.h | 1 +
>> 4 files changed, 641 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> b/drivers/net/ethernet/stmicro/stmmac/Makefile index ac4d562..73c2715
>> 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o
>> ring_mode.o \
>>
>> obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
>> stmmac-platform-objs:= stmmac_platform.o dwmac-meson.o dwmac-sunxi.o \
>> - dwmac-sti.o dwmac-socfpga.o
>> + dwmac-sti.o dwmac-socfpga.o dwmac-rk.o
>>
>> obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
>> stmmac-pci-objs:= stmmac_pci.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c new file mode 100644
>> index 0000000..870563f
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -0,0 +1,636 @@
>> +/**
>> + * dwmac-rk.c - Rockchip RK3288 DWMAC specific glue layer
>> + *
>> + * Copyright (C) 2014 Chen-Zhi (Roger Chen)
>> + *
>> + * Chen-Zhi (Roger Chen) <roger.chen@rock-chips.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/stmmac.h>
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/phy.h>
>> +#include <linux/of_net.h>
>> +#include <linux/gpio.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/of_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/delay.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/regmap.h>
>> +
>> +struct rk_priv_data {
>> + struct platform_device *pdev;
>> + int phy_iface;
>> + bool power_ctrl_by_pmu;
>> + char pmu_regulator[32];
>> + int pmu_enable_level;
>> +
>> + int power_io;
>> + int power_io_level;
>> + int reset_io;
>> + int reset_io_level;
>> + int phyirq_io;
>> + int phyirq_io_level;
>> +
>> + bool clk_enabled;
>> + bool clock_input;
>> +
>> + struct clk *clk_mac;
>> + struct clk *clk_mac_pll;
>> + struct clk *gmac_clkin;
>> + struct clk *mac_clk_rx;
>> + struct clk *mac_clk_tx;
>> + struct clk *clk_mac_ref;
>> + struct clk *clk_mac_refout;
>> + struct clk *aclk_mac;
>> + struct clk *pclk_mac;
>> +
>> + int tx_delay;
>> + int rx_delay;
>> +
>> + struct regmap *grf;
>> +};
>> +
>> +#define RK3288_GRF_SOC_CON1 0x0248
>> +#define RK3288_GRF_SOC_CON3 0x0250
>> +#define RK3288_GRF_GPIO3D_E 0x01ec
>> +#define RK3288_GRF_GPIO4A_E 0x01f0
>> +#define RK3288_GRF_GPIO4B_E 0x01f4
> here you're using a space instead of a tab, please select one pattern either
> tabs or space but do not mix them.
>
>
>> +#define GPIO3D_2MA 0xFFFF0000
>> +#define GPIO3D_4MA 0xFFFF5555
>> +#define GPIO3D_8MA 0xFFFFAAAA
>> +#define GPIO3D_12MA 0xFFFFFFFF
>> +
>> +#define GPIO4A_2MA 0xFFFF0000
>> +#define GPIO4A_4MA 0xFFFF5555
>> +#define GPIO4A_8MA 0xFFFFAAAA
>> +#define GPIO4A_12MA 0xFFFFFFFF
> see comment about pin settings below
>
>
>> +
>> +#define GRF_BIT(nr) (BIT(nr) | BIT(nr+16))
>> +#define GRF_CLR_BIT(nr) (BIT(nr+16))
>> +
>> +#define GPIO4B_2_2MA (GRF_CLR_BIT(2) | GRF_CLR_BIT(3))
>> +#define GPIO4B_2_4MA (GRF_BIT(2) | GRF_CLR_BIT(3))
>> +#define GPIO4B_2_8MA (GRF_CLR_BIT(2) | GRF_BIT(3))
>> +#define GPIO4B_2_12MA (GRF_BIT(2) | GRF_BIT(3))
>> +
>> +/*RK3288_GRF_SOC_CON1*/
>> +#define GMAC_PHY_INTF_SEL_RGMII (GRF_BIT(6) | GRF_CLR_BIT(7) |
>> GRF_CLR_BIT(8))
>> +#define GMAC_PHY_INTF_SEL_RMII (GRF_CLR_BIT(6) |
>> GRF_CLR_BIT(7) | GRF_BIT(8))
>> +#define GMAC_FLOW_CTRL GRF_BIT(9)
>> +#define GMAC_FLOW_CTRL_CLR GRF_CLR_BIT(9)
>> +#define GMAC_SPEED_10M GRF_CLR_BIT(10)
>> +#define GMAC_SPEED_100M GRF_BIT(10)
>> +#define GMAC_RMII_CLK_25M GRF_BIT(11)
>> +#define GMAC_RMII_CLK_2_5M GRF_CLR_BIT(11)
>> +#define GMAC_CLK_125M (GRF_CLR_BIT(12) | GRF_CLR_BIT(13))
>> +#define GMAC_CLK_25M (GRF_BIT(12) | GRF_BIT(13))
>> +#define GMAC_CLK_2_5M (GRF_CLR_BIT(12) | GRF_BIT(13))
>> +#define GMAC_RMII_MODE GRF_BIT(14)
>> +#define GMAC_RMII_MODE_CLR GRF_CLR_BIT(14)
>> +
>> +/*RK3288_GRF_SOC_CON3*/
>> +#define GMAC_TXCLK_DLY_ENABLE GRF_BIT(14)
>> +#define GMAC_TXCLK_DLY_DISABLE GRF_CLR_BIT(14)
>> +#define GMAC_RXCLK_DLY_ENABLE GRF_BIT(15)
>> +#define GMAC_RXCLK_DLY_DISABLE GRF_CLR_BIT(15)
>> +#define GMAC_CLK_RX_DL_CFG(val) ((0x7F<<7<<16) | (val<<7))
>> +#define GMAC_CLK_TX_DL_CFG(val) ((0x7F<<16) | (val))
> again mixed tabs and spaces as delimiters.
>
> Also the _CFG macros are not well abstracted. You could take a look at the
> HIWORD_UPDATE macro in drivers/clk/rockchip/clk.h:
>
> #define GMAC_CLK_DL_MASK 0x7f
> #define GMAC_CLK_RX_DL_CFG(val) HIWORD_UPDATE(val, GMAC_CLK_DL_MASK, 7)
> #define GMAC_CLK_TX_DL_CFG(val) HIWORD_UPDATE(val, GMAC_CLK_DL_MASK, 0)
>
>
>> +
>> +static void set_to_rgmii(struct rk_priv_data *bsp_priv,
>> + int tx_delay, int rx_delay)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
>> + return;
>> + }
>> +
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_PHY_INTF_SEL_RGMII | GMAC_RMII_MODE_CLR);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON3,
>> + GMAC_RXCLK_DLY_ENABLE | GMAC_TXCLK_DLY_ENABLE |
>> + GMAC_CLK_RX_DL_CFG(rx_delay) |
>> + GMAC_CLK_TX_DL_CFG(tx_delay));
>> + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO3D_E, GPIO3D_12MA);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4A_E, GPIO4A_12MA);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4B_E, GPIO4B_2_12MA);
> please don't write to parts controlled by other drivers - here the drive
> strength settings of pins is controlled by the pinctrl driver. Instead you can
> just set the drive-strength in the pinctrl settings.
>
>
>> +
>> + pr_debug("%s: tx delay=0x%x; rx delay=0x%x;\n",
>> + __func__, tx_delay, rx_delay);
>> +}
>> +
>> +static void set_to_rmii(struct rk_priv_data *bsp_priv)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
> you have a device-reference in rk_priv_data, so you could use dev_err here.
> Same for all other pr_err/pr_debug/pr_* calls in this file.
>
>
>> + return;
>> + }
>> +
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_PHY_INTF_SEL_RMII);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_RMII_MODE);
> these two could be combined?
>
>
>> +}
>> +
>> +static void set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
>> + return;
>> + }
>> +
>> + if (speed == 10)
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, GMAC_CLK_2_5M);
>> + else if (speed == 100)
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, GMAC_CLK_25M);
>> + else if (speed == 1000)
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, GMAC_CLK_125M);
>> + else
>> + pr_err("unknown speed value for RGMII! speed=%d", speed);
>> +}
>> +
>> +static void set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
>> +{
>> + if (IS_ERR(bsp_priv->grf)) {
>> + pr_err("%s: Missing rockchip,grf property\n", __func__);
>> + return;
>> + }
>> +
>> + if (speed == 10) {
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_RMII_CLK_2_5M);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_SPEED_10M);
> combine into one write?
>
>
>> + } else if (speed == 100) {
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_RMII_CLK_25M);
>> + regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
>> + GMAC_SPEED_100M);
> combine into one write?
>
>
>> + } else {
>> + pr_err("unknown speed value for RMII! speed=%d", speed);
>> + }
>> +}
>> +
>> +#define MAC_CLK_RX "mac_clk_rx"
>> +#define MAC_CLK_TX "mac_clk_tx"
>> +#define CLK_MAC_REF "clk_mac_ref"
>> +#define CLK_MAC_REF_OUT "clk_mac_refout"
>> +#define CLK_MAC_PLL "clk_mac_pll"
>> +#define ACLK_MAC "aclk_mac"
>> +#define PCLK_MAC "pclk_mac"
>> +#define MAC_CLKIN "ext_gmac"
>> +#define CLK_MAC "stmmaceth"
> why the need to extra constants for the clock names and not use the real names
> directly like most other drivers do?
>
>
>> +
>> +static int gmac_clk_init(struct rk_priv_data *bsp_priv)
>> +{
>> + struct device *dev = &bsp_priv->pdev->dev;
>> +
>> + bsp_priv->clk_enabled = false;
>> +
>> + bsp_priv->mac_clk_rx = clk_get(dev, MAC_CLK_RX);
>> + if (IS_ERR(bsp_priv->mac_clk_rx))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, MAC_CLK_RX);
>> +
>> + bsp_priv->mac_clk_tx = clk_get(dev, MAC_CLK_TX);
>> + if (IS_ERR(bsp_priv->mac_clk_tx))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, MAC_CLK_TX);
>> +
>> + bsp_priv->clk_mac_ref = clk_get(dev, CLK_MAC_REF);
>> + if (IS_ERR(bsp_priv->clk_mac_ref))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, CLK_MAC_REF);
>> +
>> + bsp_priv->clk_mac_refout = clk_get(dev, CLK_MAC_REF_OUT);
>> + if (IS_ERR(bsp_priv->clk_mac_refout))
>> + pr_warn("%s: warning:cannot get clock %s\n",
>> + __func__, CLK_MAC_REF_OUT);
>> +
>> + bsp_priv->aclk_mac = clk_get(dev, ACLK_MAC);
>> + if (IS_ERR(bsp_priv->aclk_mac))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, ACLK_MAC);
>> +
>> + bsp_priv->pclk_mac = clk_get(dev, PCLK_MAC);
>> + if (IS_ERR(bsp_priv->pclk_mac))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, PCLK_MAC);
>> +
>> + bsp_priv->clk_mac_pll = clk_get(dev, CLK_MAC_PLL);
>> + if (IS_ERR(bsp_priv->clk_mac_pll))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, CLK_MAC_PLL);
>> +
>> + bsp_priv->gmac_clkin = clk_get(dev, MAC_CLKIN);
>> + if (IS_ERR(bsp_priv->gmac_clkin))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, MAC_CLKIN);
>> +
>> + bsp_priv->clk_mac = clk_get(dev, CLK_MAC);
>> + if (IS_ERR(bsp_priv->clk_mac))
>> + pr_warn("%s: warning: cannot get clock %s\n",
>> + __func__, CLK_MAC);
> there is not clk_put in the _remove case ... maybe you could simply use
> devm_clk_get here so that all clocks are put on device removal.
>
> Also you're warning on every missing clock. Below it looks like you need a
> different set of them for rgmii and rmii, so maybe you should simply error out
> when core clocks for the selected phy-mode are missing.
>
>
>> +
>> + if (bsp_priv->clock_input) {
>> + pr_info("%s: clock input from PHY\n", __func__);
>> + } else {
>> + if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
>> + clk_set_rate(bsp_priv->clk_mac_pll, 50000000);
>> +
>> + clk_set_parent(bsp_priv->clk_mac, bsp_priv->clk_mac_pll);
> why the explicit reparenting. The common clock-framework is intelligent enough
> to select the best suitable parent.
>
> In general I'm thinking the clock-handling inside this driver should be
> simplyfied, as the common-clock framework can handle most cases itself. I.e. if
> a 125MHz external clock is present and so on. But haven't looked to deep yet.
>
>
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + int phy_iface = phy_iface = bsp_priv->phy_iface;
>> +
>> + if (enable) {
>> + if (!bsp_priv->clk_enabled) {
>> + if (phy_iface == PHY_INTERFACE_MODE_RMII) {
>> + if (!IS_ERR(bsp_priv->mac_clk_rx))
>> + clk_prepare_enable(
>> + bsp_priv->mac_clk_rx);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_ref))
>> + clk_prepare_enable(
>> + bsp_priv->clk_mac_ref);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_refout))
>> + clk_prepare_enable(
>> + bsp_priv->clk_mac_refout);
>> + }
>> +
>> + if (!IS_ERR(bsp_priv->aclk_mac))
>> + clk_prepare_enable(bsp_priv->aclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->pclk_mac))
>> + clk_prepare_enable(bsp_priv->pclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->mac_clk_tx))
>> + clk_prepare_enable(bsp_priv->mac_clk_tx);
>> +
>> + /**
>> + * if (!IS_ERR(bsp_priv->clk_mac))
>> + * clk_prepare_enable(bsp_priv->clk_mac);
>> + */
>> + mdelay(5);
>> + bsp_priv->clk_enabled = true;
>> + }
>> + } else {
>> + if (bsp_priv->clk_enabled) {
>> + if (phy_iface == PHY_INTERFACE_MODE_RMII) {
>> + if (!IS_ERR(bsp_priv->mac_clk_rx))
>> + clk_disable_unprepare(
>> + bsp_priv->mac_clk_rx);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_ref))
>> + clk_disable_unprepare(
>> + bsp_priv->clk_mac_ref);
>> +
>> + if (!IS_ERR(bsp_priv->clk_mac_refout))
>> + clk_disable_unprepare(
>> + bsp_priv->clk_mac_refout);
>> + }
>> +
>> + if (!IS_ERR(bsp_priv->aclk_mac))
>> + clk_disable_unprepare(bsp_priv->aclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->pclk_mac))
>> + clk_disable_unprepare(bsp_priv->pclk_mac);
>> +
>> + if (!IS_ERR(bsp_priv->mac_clk_tx))
>> + clk_disable_unprepare(bsp_priv->mac_clk_tx);
>> + /**
>> + * if (!IS_ERR(bsp_priv->clk_mac))
>> + * clk_disable_unprepare(bsp_priv->clk_mac);
>> + */
>> + bsp_priv->clk_enabled = false;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int power_on_by_pmu(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + struct regulator *ldo;
>> + char *ldostr = bsp_priv->pmu_regulator;
>> + int ret;
>> +
>> + if (!ldostr) {
>> + pr_err("%s: no ldo found\n", __func__);
>> + return -1;
>> + }
>> +
>> + ldo = regulator_get(NULL, ldostr);
>> + if (!ldo) {
>> + pr_err("\n%s get ldo %s failed\n", __func__, ldostr);
>> + } else {
>> + if (enable) {
>> + if (!regulator_is_enabled(ldo)) {
>> + regulator_set_voltage(ldo, 3300000, 3300000);
>> + ret = regulator_enable(ldo);
>> + if (ret != 0)
>> + pr_err("%s: fail to enable %s\n",
>> + __func__, ldostr);
>> + else
>> + pr_info("turn on ldo done.\n");
>> + } else {
>> + pr_warn("%s is enabled before enable", ldostr);
>> + }
>> + } else {
>> + if (regulator_is_enabled(ldo)) {
>> + ret = regulator_disable(ldo);
>> + if (ret != 0)
>> + pr_err("%s: fail to disable %s\n",
>> + __func__, ldostr);
>> + else
>> + pr_info("turn off ldo done.\n");
>> + } else {
>> + pr_warn("%s is disabled before disable",
>> + ldostr);
>> + }
>> + }
>> + regulator_put(ldo);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int power_on_by_gpio(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + if (enable) {
>> + /*power on*/
>> + if (gpio_is_valid(bsp_priv->power_io))
>> + gpio_direction_output(bsp_priv->power_io,
>> + bsp_priv->power_io_level);
>> + } else {
>> + /*power off*/
>> + if (gpio_is_valid(bsp_priv->power_io))
>> + gpio_direction_output(bsp_priv->power_io,
>> + !bsp_priv->power_io_level);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
>> +{
>> + int ret = -1;
>> +
>> + pr_info("Ethernet PHY power %s\n", enable == 1 ? "on" : "off");
>> +
>> + if (bsp_priv->power_ctrl_by_pmu)
>> + ret = power_on_by_pmu(bsp_priv, enable);
>> + else
>> + ret = power_on_by_gpio(bsp_priv, enable);
> this looks wrong. This should always be a regulator. Even a regulator + switch
> controlled by a gpio can still be modelled as regulator, so that you don't
> need this switch and assorted special handling - so just use the regulator API
>
In some case, it would be a switching circuit to control the power for PHY.
All I need to do is to control a GPIO to make switch on/off. So...
>> +
>> + if (enable) {
>> + /*reset*/
>> + if (gpio_is_valid(bsp_priv->reset_io)) {
>> + gpio_direction_output(bsp_priv->reset_io,
>> + bsp_priv->reset_io_level);
>> + mdelay(5);
>> + gpio_direction_output(bsp_priv->reset_io,
>> + !bsp_priv->reset_io_level);
>> + }
>> + mdelay(30);
>> +
>> + } else {
>> + /*pull down reset*/
>> + if (gpio_is_valid(bsp_priv->reset_io)) {
>> + gpio_direction_output(bsp_priv->reset_io,
>> + bsp_priv->reset_io_level);
>> + }
>> + }
> I'm not sure yet if it would be better to use the reset framework for this.
> While it says it is also meant for reset-gpios, there does not seem a driver
> for this to exist yet.
>
What should I do?
>
>> +
>> + return ret;
>> +}
>> +
>> +#define GPIO_PHY_POWER "gmac_phy_power"
>> +#define GPIO_PHY_RESET "gmac_phy_reset"
>> +#define GPIO_PHY_IRQ "gmac_phy_irq"
> again I don't understand why these constants are necessary
>
>> +
>> +static void *rk_gmac_setup(struct platform_device *pdev)
>> +{
>> + struct rk_priv_data *bsp_priv;
>> + struct device *dev = &pdev->dev;
>> + enum of_gpio_flags flags;
>> + int ret;
>> + const char *strings = NULL;
>> + int value;
>> + int irq;
>> +
>> + bsp_priv = devm_kzalloc(dev, sizeof(*bsp_priv), GFP_KERNEL);
>> + if (!bsp_priv)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + bsp_priv->phy_iface = of_get_phy_mode(dev->of_node);
>> +
>> + ret = of_property_read_string(dev->of_node, "pmu_regulator", &strings);
>> + if (ret) {
>> + pr_err("%s: Can not read property: pmu_regulator.\n", __func__);
>> + bsp_priv->power_ctrl_by_pmu = false;
>> + } else {
>> + pr_info("%s: ethernet phy power controlled by pmu(%s).\n",
>> + __func__, strings);
>> + bsp_priv->power_ctrl_by_pmu = true;
>> + strcpy(bsp_priv->pmu_regulator, strings);
>> + }
> There is a generic regulator-dt-binding for regulator-consumers available
> which you should of course use.
>
The same explanation as above
>> +
>> + ret = of_property_read_u32(dev->of_node, "pmu_enable_level", &value);
>> + if (ret) {
>> + pr_err("%s: Can not read property: pmu_enable_level.\n",
>> + __func__);
>> + bsp_priv->power_ctrl_by_pmu = false;
>> + } else {
>> + pr_info("%s: PHY power controlled by pmu(level = %s).\n",
>> + __func__, (value == 1) ? "HIGH" : "LOW");
>> + bsp_priv->power_ctrl_by_pmu = true;
>> + bsp_priv->pmu_enable_level = value;
>> + }
> What is this used for? Enabling should of course be done via regulator_enable
> and disabling using regulator_disable.
>
>
>> +
>> + ret = of_property_read_string(dev->of_node, "clock_in_out", &strings);
>> + if (ret) {
>> + pr_err("%s: Can not read property: clock_in_out.\n", __func__);
>> + bsp_priv->clock_input = true;
>> + } else {
>> + pr_info("%s: clock input or output? (%s).\n",
>> + __func__, strings);
>> + if (!strcmp(strings, "input"))
>> + bsp_priv->clock_input = true;
>> + else
>> + bsp_priv->clock_input = false;
>> + }
>> +
>> + ret = of_property_read_u32(dev->of_node, "tx_delay", &value);
>> + if (ret) {
>> + bsp_priv->tx_delay = 0x30;
>> + pr_err("%s: Can not read property: tx_delay.", __func__);
>> + pr_err("%s: set tx_delay to 0x%x\n",
>> + __func__, bsp_priv->tx_delay);
>> + } else {
>> + pr_info("%s: TX delay(0x%x).\n", __func__, value);
>> + bsp_priv->tx_delay = value;
>> + }
>> +
>> + ret = of_property_read_u32(dev->of_node, "rx_delay", &value);
>> + if (ret) {
>> + bsp_priv->rx_delay = 0x10;
>> + pr_err("%s: Can not read property: rx_delay.", __func__);
>> + pr_err("%s: set rx_delay to 0x%x\n",
>> + __func__, bsp_priv->rx_delay);
>> + } else {
>> + pr_info("%s: RX delay(0x%x).\n", __func__, value);
>> + bsp_priv->rx_delay = value;
>> + }
>> +
>> + bsp_priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "rockchip,grf");
>> + bsp_priv->phyirq_io =
>> + of_get_named_gpio_flags(dev->of_node,
>> + "phyirq-gpio", 0, &flags);
>> + bsp_priv->phyirq_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
>> +
>> + bsp_priv->reset_io =
>> + of_get_named_gpio_flags(dev->of_node,
>> + "reset-gpio", 0, &flags);
>> + bsp_priv->reset_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
>> +
>> + bsp_priv->power_io =
>> + of_get_named_gpio_flags(dev->of_node, "power-gpio", 0, &flags);
>> + bsp_priv->power_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
>> +
>> + /*power*/
>> + if (!gpio_is_valid(bsp_priv->power_io)) {
>> + pr_err("%s: Failed to get GPIO %s.\n",
>> + __func__, "power-gpio");
>> + } else {
>> + ret = gpio_request(bsp_priv->power_io, GPIO_PHY_POWER);
>> + if (ret)
>> + pr_err("%s: ERROR: Failed to request GPIO %s.\n",
>> + __func__, GPIO_PHY_POWER);
>> + }
> When everything power-related is handled using the regulator api, you don't
> need this
The same explanation as above
>
>> +
>> + if (!gpio_is_valid(bsp_priv->reset_io)) {
>> + pr_err("%s: ERROR: Get reset-gpio failed.\n", __func__);
>> + } else {
>> + ret = gpio_request(bsp_priv->reset_io, GPIO_PHY_RESET);
>> + if (ret)
>> + pr_err("%s: ERROR: Failed to request GPIO %s.\n",
>> + __func__, GPIO_PHY_RESET);
>> + }
>> +
>> + if (bsp_priv->phyirq_io > 0) {
> This is more for my understanding: why does the mac driver need to handle the
> phy interrupt - but I might be overlooking something.
>
phy interrupt is not mandatory. In most of the time, in order to find
something happen in PHY, for example,
link is up or down, we just use polling method to read the phy's
register in a timer.
Buf if phy interrupt is in use, when link is up or down, phy interrupt
pin will be assert to inform the CPU.
I just implement the driver for phy interrupt gpio, not enable it as
default.
>> + struct plat_stmmacenet_data *plat_dat;
>> +
>> + pr_info("PHY irq in use\n");
>> + ret = gpio_request(bsp_priv->phyirq_io, GPIO_PHY_IRQ);
>> + if (ret < 0) {
>> + pr_warn("%s: Failed to request GPIO %s\n",
>> + __func__, GPIO_PHY_IRQ);
>> + goto goon;
>> + }
>> +
>> + ret = gpio_direction_input(bsp_priv->phyirq_io);
>> + if (ret < 0) {
>> + pr_err("%s, Failed to set input for GPIO %s\n",
>> + __func__, GPIO_PHY_IRQ);
>> + gpio_free(bsp_priv->phyirq_io);
>> + goto goon;
>> + }
>> +
>> + irq = gpio_to_irq(bsp_priv->phyirq_io);
>> + if (irq < 0) {
>> + ret = irq;
>> + pr_err("Failed to set irq for %s\n",
>> + GPIO_PHY_IRQ);
>> + gpio_free(bsp_priv->phyirq_io);
>> + goto goon;
>> + }
>> +
>> + plat_dat = dev_get_platdata(&pdev->dev);
>> + if (plat_dat)
>> + plat_dat->mdio_bus_data->probed_phy_irq = irq;
>> + else
>> + pr_err("%s: plat_data is NULL\n", __func__);
>> + }
>> +
>> +goon:
>> + /*rmii or rgmii*/
>> + if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RGMII) {
>> + pr_info("%s: init for RGMII\n", __func__);
>> + set_to_rgmii(bsp_priv, bsp_priv->tx_delay, bsp_priv->rx_delay);
>> + } else if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII) {
>> + pr_info("%s: init for RMII\n", __func__);
>> + set_to_rmii(bsp_priv);
>> + } else {
>> + pr_err("%s: ERROR: NO interface defined!\n", __func__);
>> + }
>> +
>> + bsp_priv->pdev = pdev;
>> +
>> + gmac_clk_init(bsp_priv);
>> +
>> + return bsp_priv;
>> +}
>> +
>> +static int rk_gmac_init(struct platform_device *pdev, void *priv)
>> +{
>> + struct rk_priv_data *bsp_priv = priv;
>> + int ret;
>> +
>> + ret = phy_power_on(bsp_priv, true);
>> + if (ret)
>> + return ret;
>> +
>> + ret = gmac_clk_enable(bsp_priv, true);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static void rk_gmac_exit(struct platform_device *pdev, void *priv)
>> +{
>> + struct rk_priv_data *gmac = priv;
>> +
>> + phy_power_on(gmac, false);
>> + gmac_clk_enable(gmac, false);
>> +}
>> +
>> +static void rk_fix_speed(void *priv, unsigned int speed)
>> +{
>> + struct rk_priv_data *bsp_priv = priv;
>> +
>> + if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RGMII)
>> + set_rgmii_speed(bsp_priv, speed);
>> + else if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
>> + set_rmii_speed(bsp_priv, speed);
>> + else
>> + pr_err("unsupported interface %d", bsp_priv->phy_iface);
>> +}
>> +
>> +const struct stmmac_of_data rk_gmac_data = {
>> + .has_gmac = 1,
>> + .fix_mac_speed = rk_fix_speed,
>> + .setup = rk_gmac_setup,
>> + .init = rk_gmac_init,
>> + .exit = rk_gmac_exit,
>> +};
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index
>> 15814b7..b4dee96 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -33,6 +33,7 @@
>>
>> static const struct of_device_id stmmac_dt_ids[] = {
>> /* SoC specific glue layers should come before generic bindings */
>> + { .compatible = "rockchip,rk3288-gmac", .data = &rk_gmac_data},
> please name that rk3288_gmac_data [of course the other occurences too]
> It makes it easier to see which soc it is meant for and it's also not save to
> assume that the next one will use the same register + bit positions in the
> grf.
>
>
>> { .compatible = "amlogic,meson6-dwmac", .data = &meson6_dwmac_data},
>> { .compatible = "allwinner,sun7i-a20-gmac", .data = &sun7i_gmac_data},
>> { .compatible = "st,stih415-dwmac", .data = &stih4xx_dwmac_data},
>> @@ -291,6 +292,8 @@ static int stmmac_pltfr_probe(struct platform_device
>> *pdev) return -ENOMEM;
>> }
>>
>> + pdev->dev.platform_data = plat_dat;
>> +
>> ret = stmmac_probe_config_dt(pdev, plat_dat, &mac);
>> if (ret) {
>> pr_err("%s: main dt probe failed", __func__);
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h index
>> 25dd1f7..32a0516 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
>> @@ -24,5 +24,6 @@ extern const struct stmmac_of_data sun7i_gmac_data;
>> extern const struct stmmac_of_data stih4xx_dwmac_data;
>> extern const struct stmmac_of_data stid127_dwmac_data;
>> extern const struct stmmac_of_data socfpga_gmac_data;
>> +extern const struct stmmac_of_data rk_gmac_data;
>>
>> #endif /* __STMMAC_PLATFORM_H__ */
>
>
^ permalink raw reply
* Re: [PATCH] net: less interrupt masking in NAPI
From: Eric Dumazet @ 2014-12-03 7:41 UTC (permalink / raw)
To: Yang Yingliang; +Cc: David Miller, netdev, willemb
In-Reply-To: <547EBC66.4040301@huawei.com>
On Wed, 2014-12-03 at 15:31 +0800, Yang Yingliang wrote:
> This patch can resolve my performance problem.
> Will/Can this patch queue for stable ?
Hmm.. please give us more details, I am surprised it can have
a big impact.
^ permalink raw reply
* Re: [PATCH net-next v3] rtnetlink: delay RTM_DELLINK notification until after ndo_uninit()
From: Eric Dumazet @ 2014-12-03 7:39 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: netdev, David Miller, Eric Dumazet, Roopa Prabhu, Toshiaki Makita
In-Reply-To: <1417591497-24246-1-git-send-email-maheshb@google.com>
On Tue, 2014-12-02 at 23:24 -0800, Mahesh Bandewar wrote:
> +EXPORT_SYMBOL(rtmsg_ifinfo_build_skb);
> +
> +void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
> +{
> + struct net *net = dev_net(dev);
> +
> + rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
> +}
> +EXPORT_SYMBOL(rtmsg_ifinfo_send);
> +
> +void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
> + gfp_t flags)
> +{
> + struct sk_buff *skb;
> +
> + skb = rtmsg_ifinfo_build_skb(type, dev, change, flags);
> + if (skb)
> + rtmsg_ifinfo_send(skb, dev, flags);
> }
> EXPORT_SYMBOL(rtmsg_ifinfo);
One last point :
Only rtmsg_ifinfo() needs the EXPORT_SYMBOL(...)
rtmsg_ifinfo_build_skb() and rtmsg_ifinfo_send() are used from core networking stack,
not a module.
^ permalink raw reply
* Re: [PATCH] net: less interrupt masking in NAPI
From: Yang Yingliang @ 2014-12-03 7:31 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: netdev, willemb
In-Reply-To: <20141103.122538.387451917276174830.davem@davemloft.net>
On 2014/11/4 1:25, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sun, 02 Nov 2014 06:19:33 -0800
>
>> From: Eric Dumazet <edumazet@google.com>
>>
>> net_rx_action() can mask irqs a single time to transfert sd->poll_list
>> into a private list, for a very short duration.
>>
>> Then, napi_complete() can avoid masking irqs again,
>> and net_rx_action() only needs to mask irq again in slow path.
>>
>> This patch removes 2 couples of irq mask/unmask per typical NAPI run,
>> more if multiple napi were triggered.
>>
>> Note this also allows to give control back to caller (do_softirq())
>> more often, so that other softirq handlers can be called a bit earlier,
>> or ksoftirqd can be wakeup earlier under pressure.
>>
>> This was developed while testing an alternative to RX interrupt
>> mitigation to reduce latencies while keeping or improving GRO
>> aggregation on fast NIC.
>>
>> Idea is to test napi->gro_list at the end of a napi->poll() and
>> reschedule one NAPI poll, but after servicing a full round of
>> softirqs (timers, TX, rcu, ...). This will be allowed only if softirq
>> is currently serviced by idle task or ksoftirqd, and resched not needed.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Also applied, thanks Eric.
This patch can resolve my performance problem.
Will/Can this patch queue for stable ?
Regards,
Yang
^ permalink raw reply
* [PATCH net-next v3] rtnetlink: delay RTM_DELLINK notification until after ndo_uninit()
From: Mahesh Bandewar @ 2014-12-03 7:24 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Roopa Prabhu, Toshiaki Makita,
Mahesh Bandewar
The commit 56bfa7ee7c ("unregister_netdevice : move RTM_DELLINK to
until after ndo_uninit") tried to do this ealier but while doing so
it created a problem. Unfortunately the delayed rtmsg_ifinfo() also
delayed call to fill_info(). So this translated into asking driver
to remove private state and then query it's private state. This
could have catastropic consequences.
This change breaks the rtmsg_ifinfo() into two parts - one takes the
precise snapshot of the device by called fill_info() before calling
the ndo_uninit() and the second part sends the notification using
collected snapshot.
It was brought to notice when last link is deleted from an ipvlan device
when it has free-ed the port and the subsequent .fill_info() call is
trying to get the info from the port.
kernel: [ 255.139429] ------------[ cut here ]------------
kernel: [ 255.139439] WARNING: CPU: 12 PID: 11173 at net/core/rtnetlink.c:2238 rtmsg_ifinfo+0x100/0x110()
kernel: [ 255.139493] Modules linked in: ipvlan bonding w1_therm ds2482 wire cdc_acm ehci_pci ehci_hcd i2c_dev i2c_i801 i2c_core msr cpuid bnx2x ptp pps_core mdio libcrc32c
kernel: [ 255.139513] CPU: 12 PID: 11173 Comm: ip Not tainted 3.18.0-smp-DEV #167
kernel: [ 255.139514] Hardware name: Intel RML,PCH/Ibis_QC_18, BIOS 1.0.10 05/15/2012
kernel: [ 255.139515] 0000000000000009 ffff880851b6b828 ffffffff815d87f4 00000000000000e0
kernel: [ 255.139516] 0000000000000000 ffff880851b6b868 ffffffff8109c29c 0000000000000000
kernel: [ 255.139518] 00000000ffffffa6 00000000000000d0 ffffffff81aaf580 0000000000000011
kernel: [ 255.139520] Call Trace:
kernel: [ 255.139527] [<ffffffff815d87f4>] dump_stack+0x46/0x58
kernel: [ 255.139531] [<ffffffff8109c29c>] warn_slowpath_common+0x8c/0xc0
kernel: [ 255.139540] [<ffffffff8109c2ea>] warn_slowpath_null+0x1a/0x20
kernel: [ 255.139544] [<ffffffff8150d570>] rtmsg_ifinfo+0x100/0x110
kernel: [ 255.139547] [<ffffffff814f78b5>] rollback_registered_many+0x1d5/0x2d0
kernel: [ 255.139549] [<ffffffff814f79cf>] unregister_netdevice_many+0x1f/0xb0
kernel: [ 255.139551] [<ffffffff8150acab>] rtnl_dellink+0xbb/0x110
kernel: [ 255.139553] [<ffffffff8150da90>] rtnetlink_rcv_msg+0xa0/0x240
kernel: [ 255.139557] [<ffffffff81329283>] ? rhashtable_lookup_compare+0x43/0x80
kernel: [ 255.139558] [<ffffffff8150d9f0>] ? __rtnl_unlock+0x20/0x20
kernel: [ 255.139562] [<ffffffff8152cb11>] netlink_rcv_skb+0xb1/0xc0
kernel: [ 255.139563] [<ffffffff8150a495>] rtnetlink_rcv+0x25/0x40
kernel: [ 255.139565] [<ffffffff8152c398>] netlink_unicast+0x178/0x230
kernel: [ 255.139567] [<ffffffff8152c75f>] netlink_sendmsg+0x30f/0x420
kernel: [ 255.139571] [<ffffffff814e0b0c>] sock_sendmsg+0x9c/0xd0
kernel: [ 255.139575] [<ffffffff811d1d7f>] ? rw_copy_check_uvector+0x6f/0x130
kernel: [ 255.139577] [<ffffffff814e11c9>] ? copy_msghdr_from_user+0x139/0x1b0
kernel: [ 255.139578] [<ffffffff814e1774>] ___sys_sendmsg+0x304/0x310
kernel: [ 255.139581] [<ffffffff81198723>] ? handle_mm_fault+0xca3/0xde0
kernel: [ 255.139585] [<ffffffff811ebc4c>] ? destroy_inode+0x3c/0x70
kernel: [ 255.139589] [<ffffffff8108e6ec>] ? __do_page_fault+0x20c/0x500
kernel: [ 255.139597] [<ffffffff811e8336>] ? dput+0xb6/0x190
kernel: [ 255.139606] [<ffffffff811f05f6>] ? mntput+0x26/0x40
kernel: [ 255.139611] [<ffffffff811d2b94>] ? __fput+0x174/0x1e0
kernel: [ 255.139613] [<ffffffff814e2129>] __sys_sendmsg+0x49/0x90
kernel: [ 255.139615] [<ffffffff814e2182>] SyS_sendmsg+0x12/0x20
kernel: [ 255.139617] [<ffffffff815df092>] system_call_fastpath+0x12/0x17
kernel: [ 255.139619] ---[ end trace 5e6703e87d984f6b ]---
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Reported-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: David S. Miller <davem@davemloft.net>
---
v1:
Initial version
v2:
Keep the rtmsg_ifinfo() return type as it is but break the function into
two minimizing the changes all over places
v3:
Corrected an error in the code.
---
include/linux/rtnetlink.h | 5 +++++
net/core/dev.c | 12 +++++++++---
net/core/rtnetlink.c | 27 +++++++++++++++++++++++----
3 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 6cacbce1a06c..19dc0bce9c2b 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -17,6 +17,11 @@ extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
u32 id, long expires, u32 error);
void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags);
+struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
+ unsigned change, gfp_t flags);
+void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev,
+ gfp_t flags);
+
/* RTNL is used as a global lock for all changes to network configuration */
extern void rtnl_lock(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index ac4836241a96..98f6563b68b6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5925,6 +5925,8 @@ static void rollback_registered_many(struct list_head *head)
synchronize_net();
list_for_each_entry(dev, head, unreg_list) {
+ struct sk_buff *skb = NULL;
+
/* Shutdown queueing discipline. */
dev_shutdown(dev);
@@ -5934,6 +5936,11 @@ static void rollback_registered_many(struct list_head *head)
*/
call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
+ if (!dev->rtnl_link_ops ||
+ dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
+ skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U,
+ GFP_KERNEL);
+
/*
* Flush the unicast and multicast chains
*/
@@ -5943,9 +5950,8 @@ static void rollback_registered_many(struct list_head *head)
if (dev->netdev_ops->ndo_uninit)
dev->netdev_ops->ndo_uninit(dev);
- if (!dev->rtnl_link_ops ||
- dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
- rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
+ if (skb)
+ rtmsg_ifinfo_send(skb, dev, GFP_KERNEL);
/* Notifier chain MUST detach us all upper devices. */
WARN_ON(netdev_has_any_upper_dev(dev));
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b9b7dfaf202b..88787c10b8a6 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2220,8 +2220,8 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
- gfp_t flags)
+struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
+ unsigned int change, gfp_t flags)
{
struct net *net = dev_net(dev);
struct sk_buff *skb;
@@ -2239,11 +2239,30 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
kfree_skb(skb);
goto errout;
}
- rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
- return;
+ return skb;
errout:
if (err < 0)
rtnl_set_sk_err(net, RTNLGRP_LINK, err);
+ return NULL;
+}
+EXPORT_SYMBOL(rtmsg_ifinfo_build_skb);
+
+void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
+{
+ struct net *net = dev_net(dev);
+
+ rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
+}
+EXPORT_SYMBOL(rtmsg_ifinfo_send);
+
+void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
+ gfp_t flags)
+{
+ struct sk_buff *skb;
+
+ skb = rtmsg_ifinfo_build_skb(type, dev, change, flags);
+ if (skb)
+ rtmsg_ifinfo_send(skb, dev, flags);
}
EXPORT_SYMBOL(rtmsg_ifinfo);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH v3 net-next 2/2 tuntap: Increase the number of queues in tun.
From: Pankaj Gupta @ 2014-12-03 7:19 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
In-Reply-To: <1417591177-7985-1-git-send-email-pagupta@redhat.com>
Networking under kvm works best if we allocate a per-vCPU RX and TX
queue in a virtual NIC. This requires a per-vCPU queue on the host side.
It is now safe to increase the maximum number of queues.
Preceding patche: 'net: allow large number of rx queues'
made sure this won't cause failures due to high order memory
allocations. Increase it to 256: this is the max number of vCPUs
KVM supports.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Reviewed-by: David Gibson <dgibson@redhat.com>
---
drivers/net/tun.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e3fa65a..a19dc5f8 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -113,10 +113,11 @@ struct tap_filter {
unsigned char addr[FLT_EXACT_COUNT][ETH_ALEN];
};
-/* DEFAULT_MAX_NUM_RSS_QUEUES were chosen to let the rx/tx queues allocated for
- * the netdevice to be fit in one page. So we can make sure the success of
- * memory allocation. TODO: increase the limit. */
-#define MAX_TAP_QUEUES DEFAULT_MAX_NUM_RSS_QUEUES
+/* MAX_TAP_QUEUES 256 is chosen to allow rx/tx queues to be equal
+ * to max number of vCPUS in guest. Also, we are making sure here
+ * queue memory allocation do not fail.
+ */
+#define MAX_TAP_QUEUES 256
#define MAX_TAP_FLOWS 4096
#define TUN_FLOW_EXPIRE (3 * HZ)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 net-next 1/2] net: allow large number of rx queues
From: Pankaj Gupta @ 2014-12-03 7:19 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
In-Reply-To: <1417591177-7985-1-git-send-email-pagupta@redhat.com>
netif_alloc_rx_queues() uses kcalloc() to allocate memory
for "struct netdev_queue *_rx" array.
If we are doing large rx queue allocation kcalloc() might
fail, so this patch does a fallback to vzalloc().
Similar implementation is done for tx queue allocation in
netif_alloc_netdev_queues().
We avoid failure of high order memory allocation
with the help of vzalloc(), this allows us to do large
rx and tx queue allocation which in turn helps us to
increase the number of queues in tun.
As vmalloc() adds overhead on a critical network path,
__GFP_REPEAT flag is used with kzalloc() to do this fallback
only when really needed.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: David Gibson <dgibson@redhat.com>
---
net/core/dev.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index e916ba8..abe9560 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6059,17 +6059,25 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
EXPORT_SYMBOL(netif_stacked_transfer_operstate);
#ifdef CONFIG_SYSFS
+static void netif_free_rx_queues(struct net_device *dev)
+{
+ kvfree(dev->_rx);
+}
+
static int netif_alloc_rx_queues(struct net_device *dev)
{
unsigned int i, count = dev->num_rx_queues;
struct netdev_rx_queue *rx;
+ size_t sz = count * sizeof(*rx);
BUG_ON(count < 1);
- rx = kcalloc(count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
- if (!rx)
- return -ENOMEM;
-
+ rx = kzalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+ if (!rx) {
+ rx = vzalloc(sz);
+ if (!rx)
+ return -ENOMEM;
+ }
dev->_rx = rx;
for (i = 0; i < count; i++)
@@ -6698,9 +6706,8 @@ void free_netdev(struct net_device *dev)
netif_free_tx_queues(dev);
#ifdef CONFIG_SYSFS
- kfree(dev->_rx);
+ netif_free_rx_queues(dev);
#endif
-
kfree(rcu_dereference_protected(dev->ingress_queue, 1));
/* Flush device addresses */
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 net-net 0/2] Increase the limit of tuntap queues
From: Pankaj Gupta @ 2014-12-03 7:19 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
Networking under KVM works best if we allocate a per-vCPU rx and tx
queue in a virtual NIC. This requires a per-vCPU queue on the host side.
Modern physical NICs have multiqueue support for large number of queues.
To scale vNIC to run multiple queues parallel to maximum number of vCPU's
we need to increase number of queues support in tuntap.
Changes from v2:
PATCH 3: David Miller - flex array adds extra level of indirection
for preallocated array.(dropped, as flow array
is allocated using kzalloc with failover to zalloc).
Changes from v1:
PATCH 2: David Miller - sysctl changes to limit number of queues
not required for unprivileged users(dropped).
Changes from RFC
PATCH 1: Sergei Shtylyov - Add an empty line after declarations.
PATCH 2: Jiri Pirko - Do not introduce new module paramaters.
Michael.S.Tsirkin- We can use sysctl for limiting max number
of queues.
This series is to increase the number of tuntap queues. Original work is being
done by 'jasowang@redhat.com'. I am taking this 'https://lkml.org/lkml/2013/6/19/29'
patch series as a reference. As per discussion in the patch series:
There were two reasons which prevented us from increasing number of tun queues:
- The netdev_queue array in netdevice were allocated through kmalloc, which may
cause a high order memory allocation too when we have several queues.
E.g. sizeof(netdev_queue) is 320, which means a high order allocation would
happens when the device has more than 16 queues.
- We store the hash buckets in tun_struct which results a very large size of
tun_struct, this high order memory allocation fail easily when the memory is
fragmented.
The patch 60877a32bce00041528576e6b8df5abe9251fa73 increases the number of tx
queues. Memory allocation fallback to vzalloc() when kmalloc() fails.
This series tries to address following issues:
- Increase the number of netdev_queue queues for rx similarly its done for tx
queues by falling back to vzalloc() when memory allocation with kmalloc() fails.
- Increase number of queues to 256, maximum number is equal to maximum number
of vCPUS allowed in a guest.
I have done some testing to test any regression with sample program which creates
tun/tap for single queue / multiqueue device. I have also done testing with multiple
parallel Netperf sessions from guest to host for different combination of queues
and CPU's. It seems to be working fine without much increase in cpu load with the
increase in number of queues. Though i had limitation of 4 physical CPU's.
For this test vhost threads are pinned to separate CPU's. Below are the results:
Host kernel: 3.18.rc4, Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz, 4 CPUS
NIC : Ethernet controller: Intel Corporation 82579LM Gigabit Network
Patch Applied %usr %nice %sys %iowait %irq %soft %steal %guest %gnice %idle throughput
Single Queue
-------------
Before :all 7.94 0.01 1.79 3.00 0.26 0.15 0.00 3.21 0.00 83.64 64924.94
After :all 2.15 0.00 0.82 2.21 0.08 0.13 0.00 0.83 0.00 93.79 68799.88
2 Queues
Before :all 6.75 0.06 1.91 3.93 0.23 0.21 0.00 3.84 0.00 83.07 69569.30
After :all 2.12 0.00 0.92 2.51 0.08 0.15 0.00 1.19 0.00 93.02 71386.79
4 Queues
Before :all 6.09 0.05 1.88 3.83 0.22 0.22 0.00 3.74 0.00 83.98 76170.60
After :all 2.12 0.00 1.01 2.72 0.09 0.16 0.00 1.47 0.00 92.43 75492.34
8 Queues
Before :all 5.80 0.05 1.91 3.97 0.21 0.23 0.00 3.88 0.00 83.96 70843.88
After :all 2.06 0.00 1.06 2.77 0.09 0.17 0.00 1.66 0.00 92.19 74486.31
16 Queues
--------------
After :all 2.04 0.00 1.13 2.90 0.10 0.18 0.00 2.02 0.00 91.63 73227.45
Patches Summary:
net: allow large number of rx queues
tuntap: Increase the number of queues in tun
drivers/net/tun.c | 9 +++++----
net/core/dev.c | 19 +++++++++++++------
2 files changed, 18 insertions(+), 10 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next] r8152: reduce memory copy for rx
From: Eric Dumazet @ 2014-12-03 7:15 UTC (permalink / raw)
To: Hayes Wang
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2ED360A@RTITMBSV03.realtek.com.tw>
On Wed, 2014-12-03 at 07:05 +0000, Hayes Wang wrote:
> Eric Dumazet [mailto:eric.dumazet@gmail.com]
> > Sent: Wednesday, December 03, 2014 2:08 PM
> [...]
> > Better performance for what workload exactly ?
>
> I test it by using Chariot with 4 Tx and 4 Rx.
> It has about 4% improvement.
>
Have you tried using more concurrent RX flows, in a possibly lossy
environment (so that TCP is forced to queue packets in out of order
queue) ?
> > cloning in rx path has many drawbacks, with skb->truesize
> > being usually wrong.
>
> Excuse me. I find the skb_clone() would copy the
> truesize from original skb. Do you mean the value
> may not be suitable for the cloned skb?
With cloning, (skb->len / skb->truesize) will eventually be very very
small, forcing TCP stack to perform collapses (copies !!!) under
pressure.
>
> Could other method do the same thing? Or, do you
> think keeping the original one is better?
skb cloning prevents GRO and TCP coalescing from working.
netfilter might also be forced to copy whole frame in case a mangle is
needed (eg with NAT ...)
I would rather try to implement GRO, and/or using fragments instead of
pure linear skbs.
(skb->head would be around 128 or 256 bytes, and you attach to skb the
frame as a page fragment)
^ permalink raw reply
* RE: [PATCH net-next] r8152: reduce memory copy for rx
From: Hayes Wang @ 2014-12-03 7:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
In-Reply-To: <1417586858.5303.118.camel@edumazet-glaptop2.roam.corp.google.com>
Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Wednesday, December 03, 2014 2:08 PM
[...]
> Better performance for what workload exactly ?
I test it by using Chariot with 4 Tx and 4 Rx.
It has about 4% improvement.
> cloning in rx path has many drawbacks, with skb->truesize
> being usually wrong.
Excuse me. I find the skb_clone() would copy the
truesize from original skb. Do you mean the value
may not be suitable for the cloned skb?
Could other method do the same thing? Or, do you
think keeping the original one is better?
Best Regards,
Hayes
^ permalink raw reply
* RE: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Du, Fan @ 2014-12-03 6:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org, fw@strlen.de, Du, Fan
In-Reply-To: <20141202.211449.2074426768363313524.davem@davemloft.net>
>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Wednesday, December 3, 2014 1:15 PM
>To: Du, Fan
>Cc: netdev@vger.kernel.org; fw@strlen.de
>Subject: Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
>
>From: "Du, Fan" <fan.du@intel.com>
>Date: Wed, 3 Dec 2014 04:50:21 +0000
>
>> Do you have any better idea to achieve what you said besides this
>> patch approach without both fragmentation and ICMP message at the same
>> time to cater for all kinds tunnel tech?
>
>I am not obligated to figure out for you how to design a correctly implemented
>patch.
>
>But I am obligated to keep a bad change from going into the tree and that is what I
>am doing.
"bad" is not depending whether you say it or not, but what the real world needs and what
proper solution could be provided at the time being.
^ permalink raw reply
* [PATCH net] cxgb4: Update FW version string to match FW binary version 1.12.25.0
From: Hariprasad Shenai @ 2014-12-03 6:59 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, anish, nirranjan, kumaras, Hariprasad Shenai
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 3c481b2..0514b74 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -50,13 +50,13 @@
#include "cxgb4_uld.h"
#define T4FW_VERSION_MAJOR 0x01
-#define T4FW_VERSION_MINOR 0x0B
-#define T4FW_VERSION_MICRO 0x1B
+#define T4FW_VERSION_MINOR 0x0C
+#define T4FW_VERSION_MICRO 0x19
#define T4FW_VERSION_BUILD 0x00
#define T5FW_VERSION_MAJOR 0x01
-#define T5FW_VERSION_MINOR 0x0B
-#define T5FW_VERSION_MICRO 0x1B
+#define T5FW_VERSION_MINOR 0x0C
+#define T5FW_VERSION_MICRO 0x19
#define T5FW_VERSION_BUILD 0x00
#define CH_WARN(adap, fmt, ...) dev_warn(adap->pdev_dev, fmt, ## __VA_ARGS__)
--
1.7.1
^ permalink raw reply related
* [Discussion] About over-MTU-sized skb in virtualized env
From: Du Fan @ 2014-12-03 6:39 UTC (permalink / raw)
To: Florian Westphal
Cc: Thomas Graf, Michael S. Tsirkin, Jesse Gross, Flavio Leitner,
davem@davemloft.net, pshelar, netdev, dev@openvswitch.org,
Du, Fan
Sorry for resend this mail, because my company email is rejected by netdev.
Hi Florian
214 static int ip_finish_output_gso(struct sk_buff *skb)
215 {
216 netdev_features_t features;
217 struct sk_buff *segs;
218 int ret = 0;
219
220 /* common case: locally created skb or seglen is <= mtu */
221 if (((IPCB(skb)->flags & IPSKB_FORWARDED) == 0) ||
222 skb_gso_network_seglen(skb) <= ip_skb_dst_mtu(skb))
223 return ip_finish_output2(skb);
Could you please state _concrete_ reason why locally created skb
length is ＿always＿ fitting into MTU size? or why we needs this
checking.
Especially in below scenario:
http://docs.openstack.org/admin-guide-cloud/content/figures/14/a/a/common/figures/under-the-hood-scenario-1-ovs-compute.png
As many people have discuss it in this thread:
http://marc.info/?t=141715669700004&r=1&w=2
^ permalink raw reply
* [PATCH net-next] virtio-net: don't do header check for dodgy gso packets
From: Jason Wang @ 2014-12-03 6:40 UTC (permalink / raw)
To: rusty, mst, virtualization, netdev, linux-kernel
There's no need to do header check for virito-net since:
- Host set dodgy for all gso packets from guest and check the header.
- Host should prepare for all kinds of evil packets from guest, since
malicious guest can send any kinds of packet.
So this patch sets NETIF_F_GSO_ROBUST for virtio-net to skip the check.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b0bc8ea..4cd242b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1760,6 +1760,8 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
dev->hw_features |= NETIF_F_TSO_ECN;
+ dev->features |= NETIF_F_GSO_ROBUST;
+
if (gso)
dev->features |= dev->hw_features & NETIF_F_ALL_TSO;
/* (!csum && gso) case will be fixed by register_netdev() */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 net] bpf: x86: fix epilogue generation for eBPF programs
From: Z Lim @ 2014-12-03 6:38 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Eric Dumazet, Daniel Borkmann, H. Peter Anvin,
Thomas Gleixner, Ingo Molnar, Network Development, LKML
In-Reply-To: <1417301173-23691-1-git-send-email-ast@plumgrid.com>
Hi Alexei,
On Sat, Nov 29, 2014 at 2:46 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> classic BPF has a restriction that last insn is always BPF_RET.
> eBPF doesn't have BPF_RET instruction and this restriction.
> It has BPF_EXIT insn which can appear anywhere in the program
> one or more times and it doesn't have to be last insn.
Just to confirm, in valid eBPF, BPF_EXIT *must* be present at least
once, correct?
Does an eBPF JIT implementation need to check for it?
> Fix eBPF JIT to emit epilogue when first BPF_EXIT is seen
> and all other BPF_EXIT instructions will be emitted as jump.
>
> Since jump offset to epilogue is computed as:
> jmp_offset = ctx->cleanup_addr - addrs[i]
> we need to change type of cleanup_addr to signed to compute the offset as:
> (long long) ((int)20 - (int)30)
> instead of:
> (long long) ((unsigned int)20 - (int)30)
>
> Fixes: 622582786c9e ("net: filter: x86: internal BPF JIT")
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
> Note, this bug is applicable only to native eBPF programs
> which first were introduced in 3.18, so no need for stable.
>
> arm64 JIT has the same problem, but the fix is not as trivial,
> so will be done as separate patch.
I'll cook up a patch for arm64 if you haven't already done so.
Any related test case I should run through?
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-12-03 6:29 UTC (permalink / raw)
To: Kevin Zhu
Cc: Bjørn Mork, Eli Britstein, Alex Strizhevsky,
Midge Shaojun Tan, youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <547EA82F.5050500-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 7899 bytes --]
No problems.
I remain there - for anything. Especially because if we collaborate and are
able to find a good solution to this problem, then it is so much better for
all.
Tell me if I can do something useful or try something useful...
Enrico
On Wed, 3 Dec 2014, Kevin Zhu wrote:
==Date: Wed, 3 Dec 2014 07:05:37
==From: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
==To: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==Cc: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>, Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
== "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
== "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
== "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==Subject: Re: Is this 32-bit NCM?
==
==OK. I will. Thank you for everything!
==
==Regards,
==Kevin
==
==On 12/03/2014 02:00 PM, Enrico Mioso wrote:
==> Yes - I think this would be ok. You might try this with the 16-bit river first,
==> and then with the 32-bit one to see how things work.
==> I hope for the best.
==> Let us all know,
==> Enrico
==>
==>
==> On Wed, 3 Dec 2014, Kevin Zhu wrote:
==>
==> ==Date: Wed, 3 Dec 2014 06:38:27
==> ==From: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
==> ==To: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==> ==Cc: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>, Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
==> == Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
==> == Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
==> == "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
==> == "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
==> == "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==> ==Subject: Re: Is this 32-bit NCM?
==> ==
==> ==My dongle also works with the huawei driver. I think only the 32bit
==> ==format and NDP location matter. We may modify the TX function to put NTH
==> ==and NDP at the beginning of a NTB and see if it will work with the
==> ==driver cdc_ncm.
==> ==
==> ==Regards,
==> ==Kevin
==> ==
==> ==On 12/02/2014 11:28 PM, Enrico Mioso wrote:
==> ==> ... And what do you think about the source code of their ndis driver?
==> ==> We at least know now the device work with it, so we have something to mimic :D
==> ==> thank you for your work and patience Kevin.
==> ==>
==> ==> On Tue, 2 Dec 2014, Kevin Zhu wrote:
==> ==>
==> ==> ==Date: Tue, 2 Dec 2014 16:04:25
==> ==> ==From: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>
==> ==> ==To: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
==> ==> ==Cc: Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
==> ==> == Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
==> ==> == Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
==> ==> == "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
==> ==> == "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
==> ==> == "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==> ==> ==Subject: Re: Is this 32-bit NCM?
==> ==> ==
==> ==> ==I do not understand why the wSequence matters. By the way, I think I see some NDPs are right after NTH headers in the windows capture.
==> ==> ==
==> ==> ==________________________________________
==> ==> ==From: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==> ==> ==Sent: Tuesday, December 2, 2014 21:53
==> ==> ==To: Bjørn Mork
==> ==> ==Cc: Kevin Zhu; Eli Britstein; Alex Strizhevsky; Midge Shaojun Tan; youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
==> ==> ==Subject: Re: Is this 32-bit NCM?
==> ==> ==
==> ==> ==Thank you very much Bjorn.
==> ==> ==
==> ==> ==
==> ==> ==On Tue, 2 Dec 2014, Bjørn Mork wrote:
==> ==> ==
==> ==> ====Date: Tue, 2 Dec 2014 14:37:03
==> ==> ====From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
==> ==> ====To: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
==> ==> ====Cc: Kevin Zhu <Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
==> ==> ==== Eli Britstein <Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
==> ==> ==== Alex Strizhevsky <alexxst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
==> ==> ==== Midge Shaojun Tan <ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg@public.gmane.org>,
==> ==> ==== "youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
==> ==> ==== "linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
==> ==> ==== "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
==> ==> ====Subject: Re: Is this 32-bit NCM?
==> ==> ====
==> ==> ====Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
==> ==> ====
==> ==> ====> ... but out of curiosity: are NCM specs allowing to change order of things in
==> ==> ====> the package or not?
==> ==> ====> This is not to start philosofical falames or something, but to understand
==> ==> ====> better how things work. And, if they do: how much arbitrarily?
==> ==> ====
==> ==> ====Only the NTB header has a fixed location. The rest can be anywhere and
==> ==> ====in any order. Quoting from section 3 Data Transport:
==> ==> ====
==> ==> ==== "Within any given NTB, the NTH always must be first; but the other
==> ==> ==== items may occur in arbitrary order."
==> ==> ====
==> ==> ====
==> ==> ====Bjørn
==> ==> ====
==> ==> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==> ==> ==
==> ==> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==> ==> ==
==> ==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==> ==
==> ==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==> ==
==This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
==
^ permalink raw reply
* [PATCHv2 net] cxgb4: Add a check for flashing FW using ethtool
From: Hariprasad Shenai @ 2014-12-03 6:19 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, anish, nirranjan, kumaras, Hariprasad Shenai
Don't let T4 firmware flash on a T5 adapter and vice-versa
using ethtool
Based on original work by Casey Leedom <leedom@chelsio.com>
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
V2:
Use bool for return value based on review comment by Sergei Shtylyov
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 163a2a1..c623f1f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -1131,6 +1131,27 @@ unsigned int t4_flash_cfg_addr(struct adapter *adapter)
return FLASH_CFG_START;
}
+/* Return TRUE if the specified firmware matches the adapter. I.e. T4
+ * firmware for T4 adapters, T5 firmware for T5 adapters, etc. We go ahead
+ * and emit an error message for mismatched firmware to save our caller the
+ * effort ...
+ */
+static bool t4_fw_matches_chip(const struct adapter *adap,
+ const struct fw_hdr *hdr)
+{
+ /* The expression below will return FALSE for any unsupported adapter
+ * which will keep us "honest" in the future ...
+ */
+ if ((is_t4(adap->params.chip) && hdr->chip == FW_HDR_CHIP_T4) ||
+ (is_t5(adap->params.chip) && hdr->chip == FW_HDR_CHIP_T5))
+ return true;
+
+ dev_err(adap->pdev_dev,
+ "FW image (%d) is not suitable for this adapter (%d)\n",
+ hdr->chip, CHELSIO_CHIP_VERSION(adap->params.chip));
+ return false;
+}
+
/**
* t4_load_fw - download firmware
* @adap: the adapter
@@ -1170,6 +1191,8 @@ int t4_load_fw(struct adapter *adap, const u8 *fw_data, unsigned int size)
FW_MAX_SIZE);
return -EFBIG;
}
+ if (!t4_fw_matches_chip(adap, hdr))
+ return -EINVAL;
for (csum = 0, i = 0; i < size / sizeof(csum); i++)
csum += ntohl(p[i]);
@@ -3080,6 +3103,9 @@ int t4_fw_upgrade(struct adapter *adap, unsigned int mbox,
const struct fw_hdr *fw_hdr = (const struct fw_hdr *)fw_data;
int reset, ret;
+ if (!t4_fw_matches_chip(adap, fw_hdr))
+ return -EINVAL;
+
ret = t4_fw_halt(adap, mbox, force);
if (ret < 0 && !force)
return ret;
--
1.7.1
^ permalink raw reply related
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