* [PATCH v2] ipv4: dst_entry leak in ip_append_data()
From: Vasily Averin @ 2014-10-14 4:57 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, Eric Dumazet
v2: adjust the indentation of the arguments __ip_append_data() call
Fixes: 2e77d89b2fa8 ("net: avoid a pair of dst_hold()/dst_release() in ip_append_data()")
If sk_write_queue is empty ip_append_data() executes ip_setup_cork()
that "steals" dst entry from rt to cork. Later it calls __ip_append_data()
that creates skb and adds it to sk_write_queue.
If skb was added successfully following ip_push_pending_frames() call
reassign dst entries from cork to skb, and kfree_skb frees dst_entry.
However nobody frees stolen dst_entry if skb was not added into sk_write_queue.
Signed-off-by: Vasily Averin <vvs@parallels.com>
---
net/ipv4/ip_output.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index e35b712..3ba2291 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1120,6 +1120,15 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
return 0;
}
+static void ip_cork_release(struct inet_cork *cork)
+{
+ cork->flags &= ~IPCORK_OPT;
+ kfree(cork->opt);
+ cork->opt = NULL;
+ dst_release(cork->dst);
+ cork->dst = NULL;
+}
+
/*
* ip_append_data() and ip_append_page() can make one large IP datagram
* from many pieces of data. Each pieces will be holded on the socket
@@ -1152,9 +1161,14 @@ int ip_append_data(struct sock *sk, struct flowi4 *fl4,
transhdrlen = 0;
}
- return __ip_append_data(sk, fl4, &sk->sk_write_queue, &inet->cork.base,
- sk_page_frag(sk), getfrag,
- from, length, transhdrlen, flags);
+ err = __ip_append_data(sk, fl4, &sk->sk_write_queue, &inet->cork.base,
+ sk_page_frag(sk), getfrag,
+ from, length, transhdrlen, flags);
+
+ if (skb_queue_empty(&sk->sk_write_queue))
+ ip_cork_release(&inet->cork.base);
+
+ return err;
}
ssize_t ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
@@ -1304,15 +1318,6 @@ error:
return err;
}
-static void ip_cork_release(struct inet_cork *cork)
-{
- cork->flags &= ~IPCORK_OPT;
- kfree(cork->opt);
- cork->opt = NULL;
- dst_release(cork->dst);
- cork->dst = NULL;
-}
-
/*
* Combined all pending IP fragments on the socket as one IP datagram
* and push them out.
--
1.9.1
^ permalink raw reply related
* Re: [ovs-dev] [PATCH/RFC repost 7/8] ofproto: translate datapath select group action
From: Simon Horman @ 2014-10-14 4:54 UTC (permalink / raw)
To: Ben Pfaff; +Cc: Simon Horman, dev, netdev
In-Reply-To: <20141013204624.GD6806@nicira.com>
On Mon, Oct 13, 2014 at 01:46:24PM -0700, Ben Pfaff wrote:
> On Thu, Oct 09, 2014 at 10:14:36AM +0900, Simon Horman wrote:
> > On Fri, Sep 26, 2014 at 04:57:25PM -0700, Ben Pfaff wrote:
> > > On Thu, Sep 18, 2014 at 10:55:10AM +0900, Simon Horman wrote:
> > > > This patch is a prototype and has several limitations:
> > > >
> > > > * It assumes that no actions follow a select group action
> > > > because the resulting packet after a select group action may
> > > > differ depending on the bucket used. It may be possible
> > > > to address this problem using recirculation. Or to not use
> > > > the datapath select group in such situations. In any case
> > > > this patch does not solve this problem or even prevent it
> > > > from occurring.
> > >
> > > It seems like this limitation in particular is a pretty big one. Do
> > > you have a good plan in mind for how to resolve it?
> >
> > Hi Ben,
> >
> > it seems to me that this would be somewhat difficult to resolve in the
> > datapath so I propose not doing so. And I have two ideas on how to
> > resolve this problem outside of the datapath.
> >
> > 1. Recirculation
> >
> > It seems to me that it ought to be possible to handle this by
> > recirculating if actions occur after an ODP select group action.
> >
> > This could be made slightly more selective by only recirculating
> > if the execution different buckets may result in different packet
> > contents and the actions after the ODP select group action rely on
> > the packet contents (e.g. set actions do but output actions do not).
> >
> > My feeling is that this could be implemented by adding a small amount
> > of extra state to action translation in ovs-vswitchd.
> >
> > 2. Fall back to selecting buckets in ovs-vswtichd
> >
> > The idea here is to detect cases where there would be a problem
> > executing actions after an ODP select group action and in that
> > case to select buckets in ovs-vswtichd: that is use the existing bucket
> > translation code in ovs-vswtichd.
> >
> > Though this seems conceptually simpler than recirculation it
> > seems to me that it would be somewhat more difficult to implement
> > as it implies a two stage translation process: e.g. one stage to
> > determine if an ODP select group may be used; and one to perform
> > the translation.
> >
> > I seem to recall trying various two stage translation processes
> > as part some earlier unrelated work. And my recollection is that
> > the result of my previous efforts were not pretty.
> >
> > Both of the above more or less negate any benefits of ODP select group
> > action. In particular lowering flow setup cost and potentially allowing
> > complete offload of select groups from the datapath to hardware. However I
> > think that this case is not a common one as it requires both of the
> > following. And I think they are both not usual use cases.
> >
> > * Different buckets modifying packets in different ways
> > - My expectation is that it is common for buckets to be homogeneous in
> > regards to packet modifications. But perhaps this is na??ve in the
> > context of VLANs, MPLS, and similar tags that can be pushed and popped.
> > * Actions that rely on packet contents after
> > - My expectation is that it is common to use a select group to output
> > packets and that is the final action performed.
>
> I am glad that you have thought about it. Your ideas seem like a good
> start to me. Personally, approach #2, of falling back to selecting
> buckets in ovs-vswitchd, seems like a clean solution to me, although
> if it really takes multiple stages in translation then that is
> undesirable, so I hope that some clean and simple approach works out.
Thanks. I'll focus on #2 and see how far I can get.
> I think that we probably need a solution before we can apply the patch
> series, because otherwise we end up with half-working code.
Yes, I agree. It needs to be solved before it can be merged.
^ permalink raw reply
* Re: [PATCH 1/3] virtio_net: pass well-formed sgs to virtqueue_add_*()
From: Rusty Russell @ 2014-10-14 2:21 UTC (permalink / raw)
To: Michael S. Tsirkin, Paolo Bonzini; +Cc: netdev, Andy Lutomirski, virtualization
In-Reply-To: <20140907072032.GA25143@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Fri, Sep 05, 2014 at 12:40:50PM +0200, Paolo Bonzini wrote:
>> Il 03/09/2014 06:29, Rusty Russell ha scritto:
>> > + sg_init_table(rq->sg, MAX_SKB_FRAGS + 2);
>>
>> I think 2 is enough here. That said...
>>
>> > sg_set_buf(rq->sg, &hdr->hdr, sizeof hdr->hdr);
>> > -
>> > skb_to_sgvec(skb, rq->sg + 1, 0, skb->len);
>> >
>> > err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp);
>>
>> ... skb_to_sgvec will already make the sg well formed, so the
>> sg_init_table is _almost_ redundant; it is only there to remove
>> intermediate end marks. The block layer takes care to remove
>> them, but skb_to_sgvec doesn't.
sg_init_table is still needed if CONFIG_DEBUG_SG, so I don't
think it's worth it.
Thanks,
Rusty.
^ permalink raw reply
* Verify Your Account Else It Will Be Blocked
From: Webmaster @ 2014-10-14 2:18 UTC (permalink / raw)
--
Dear user,
We are undergoing maintenance therefore all accounts must be updated,
this is to reduce the number of dormant accounts.
Accounts not updated in 48 hours will be suspended.
Please follow the hyper link below to update your account
Click Here To Update Account.
http://zimbra1.my3gb.com/
Best Regards,
Administrator
^ permalink raw reply
* color box, display box, corrugated box, color card, blister card, color sleeve, hang tag, label
From: Jinghao Printing - CHINA @ 2014-10-14 1:54 UTC (permalink / raw)
Hi, this is David Wu from Shanghai, China.
We are a printing company, we can print color box, corrugated box,
label, hang tag etc.
Please let me know if you need these.
I will send you the website then.
Best regards,
David Wu
^ permalink raw reply
* Re: [PATCH v4 04/25] virtio: defer config changed notifications
From: Rusty Russell @ 2014-10-14 0:31 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: linux-s390, linux-scsi, kvm, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-5-git-send-email-mst-v4@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> Defer config changed notifications that arrive during
> probe/scan/freeze/restore.
>
> This will allow drivers to set DRIVER_OK earlier, without worrying about
> racing with config change interrupts.
>
> This change will also benefit old hypervisors (before 2009)
> that send interrupts without checking DRIVER_OK: previously,
> the callback could race with driver-specific initialization.
>
> This will also help simplify drivers.
But AFAICT you never *read* dev->config_changed.
You unconditionally trigger a config_changed event in
virtio_config_enable(). That's a bit weird, but probably OK.
How about the following change (on top of your patch). I
think the renaming is clearer, and note the added if() test in
virtio_config_enable().
If you approve, I'll fold it in.
Cheers,
Rusty.
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 2536701b098b..df598dd8c5c8 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -122,7 +122,7 @@ static void __virtio_config_changed(struct virtio_device *dev)
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
if (!dev->config_enabled)
- dev->config_changed = true;
+ dev->config_change_pending = true;
else if (drv && drv->config_changed)
drv->config_changed(dev);
}
@@ -148,8 +148,9 @@ static void virtio_config_enable(struct virtio_device *dev)
{
spin_lock_irq(&dev->config_lock);
dev->config_enabled = true;
- __virtio_config_changed(dev);
- dev->config_changed = false;
+ if (dev->config_change_pending)
+ __virtio_config_changed(dev);
+ dev->config_change_pending = false;
spin_unlock_irq(&dev->config_lock);
}
@@ -253,7 +254,7 @@ int register_virtio_device(struct virtio_device *dev)
spin_lock_init(&dev->config_lock);
dev->config_enabled = false;
- dev->config_changed = false;
+ dev->config_change_pending = false;
/* We always start by resetting the device, in case a previous
* driver messed it up. This also tests that code path a little. */
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 5636b119dc25..65261a7244fc 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -80,7 +80,7 @@ bool virtqueue_is_broken(struct virtqueue *vq);
* @index: unique position on the virtio bus
* @failed: saved value for CONFIG_S_FAILED bit (for restore)
* @config_enabled: configuration change reporting enabled
- * @config_changed: configuration change reported while disabled
+ * @config_change_pending: configuration change reported while disabled
* @config_lock: protects configuration change reporting
* @dev: underlying device.
* @id: the device type identification (used to match it with a driver).
@@ -94,7 +94,7 @@ struct virtio_device {
int index;
bool failed;
bool config_enabled;
- bool config_changed;
+ bool config_change_pending;
spinlock_t config_lock;
struct device dev;
struct virtio_device_id id;
^ permalink raw reply related
* Re: something is wrong in commit 971f10eca1 - tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Cong Wang @ 2014-10-14 0:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Krzysztof Kolasa, netdev, edumazet, David Miller
In-Reply-To: <CAHA+R7OWpjO0AbwteOAfxd1fej6BcjXR99xy+MnkZ8y+JZ8f4g@mail.gmail.com>
On Mon, Oct 13, 2014 at 4:59 PM, Cong Wang <cwang@twopensource.com> wrote:
> Probably not related with this bug, but with regarding to the
> offending commit, what's the point of the memmove() in tcp_v4_rcv()
> since ip_rcv() already clears IPCB()?
Oh, ip options are actually saved in ip_rcv_finish()... Hmm, looks scary
to play with variable-length array with memmove()....
^ permalink raw reply
* [PATCH v2 4/4] drivers: net: xgene: Add SGMII based 1GbE ethtool support
From: Iyappan Subramanian @ 2014-10-14 0:05 UTC (permalink / raw)
To: davem, romieu, netdev, devicetree
Cc: linux-arm-kernel, patches, kchudgar, Iyappan Subramanian
In-Reply-To: <1413245135-2989-1-git-send-email-isubramanian@apm.com>
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
.../net/ethernet/apm/xgene/xgene_enet_ethtool.c | 25 +++++++++++++++-------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
index c1c997b..416d6eb 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
@@ -64,16 +64,25 @@ static int xgene_get_settings(struct net_device *ndev, struct ethtool_cmd *cmd)
return -ENODEV;
return phy_ethtool_gset(phydev, cmd);
+ } else if (pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) {
+ cmd->supported = SUPPORTED_1000baseT_Full |
+ SUPPORTED_Autoneg | SUPPORTED_MII;
+ cmd->advertising = cmd->supported;
+ ethtool_cmd_speed_set(cmd, SPEED_1000);
+ cmd->duplex = DUPLEX_FULL;
+ cmd->port = PORT_MII;
+ cmd->transceiver = XCVR_INTERNAL;
+ cmd->autoneg = AUTONEG_ENABLE;
+ } else {
+ cmd->supported = SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE;
+ cmd->advertising = cmd->supported;
+ ethtool_cmd_speed_set(cmd, SPEED_10000);
+ cmd->duplex = DUPLEX_FULL;
+ cmd->port = PORT_FIBRE;
+ cmd->transceiver = XCVR_INTERNAL;
+ cmd->autoneg = AUTONEG_DISABLE;
}
- cmd->supported = SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE;
- cmd->advertising = cmd->supported;
- ethtool_cmd_speed_set(cmd, SPEED_10000);
- cmd->duplex = DUPLEX_FULL;
- cmd->port = PORT_FIBRE;
- cmd->transceiver = XCVR_EXTERNAL;
- cmd->autoneg = AUTONEG_DISABLE;
-
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v2 3/4] drivers: net: xgene: Add SGMII based 1GbE support
From: Iyappan Subramanian @ 2014-10-14 0:05 UTC (permalink / raw)
To: davem, romieu, netdev, devicetree
Cc: linux-arm-kernel, patches, kchudgar, Iyappan Subramanian
In-Reply-To: <1413245135-2989-1-git-send-email-isubramanian@apm.com>
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
drivers/net/ethernet/apm/xgene/Makefile | 2 +-
drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 3 +
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 10 +-
drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 10 +
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 389 ++++++++++++++++++++++
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h | 41 +++
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h | 3 -
7 files changed, 453 insertions(+), 5 deletions(-)
create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
diff --git a/drivers/net/ethernet/apm/xgene/Makefile b/drivers/net/ethernet/apm/xgene/Makefile
index 589b352..68be5655 100644
--- a/drivers/net/ethernet/apm/xgene/Makefile
+++ b/drivers/net/ethernet/apm/xgene/Makefile
@@ -2,6 +2,6 @@
# Makefile for APM X-Gene Ethernet Driver.
#
-xgene-enet-objs := xgene_enet_hw.o xgene_enet_xgmac.o \
+xgene-enet-objs := xgene_enet_hw.o xgene_enet_sgmac.o xgene_enet_xgmac.o \
xgene_enet_main.o xgene_enet_ethtool.o
obj-$(CONFIG_NET_XGENE) += xgene-enet.o
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
index 2efc4d9..3855858 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
@@ -44,6 +44,7 @@ static inline u32 xgene_get_bits(u32 val, u32 start, u32 end)
enum xgene_enet_rm {
RM0,
+ RM1,
RM3 = 3
};
@@ -143,6 +144,8 @@ enum xgene_enet_rm {
#define CFG_CLE_FPSEL0_SET(dst, val) xgene_set_bits(dst, val, 16, 4)
#define CFG_MACMODE_SET(dst, val) xgene_set_bits(dst, val, 18, 2)
#define CFG_WAITASYNCRD_SET(dst, val) xgene_set_bits(dst, val, 0, 16)
+#define CFG_CLE_DSTQID0(val) (val & GENMASK(11, 0))
+#define CFG_CLE_FPSEL0(val) ((val << 16) & GENMASK(19, 16))
#define ICM_CONFIG0_REG_0_ADDR 0x0400
#define ICM_CONFIG2_REG_0_ADDR 0x0410
#define RX_DV_GATE_REG_0_ADDR 0x05fc
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index 9e251ec..3c208cc 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -21,6 +21,7 @@
#include "xgene_enet_main.h"
#include "xgene_enet_hw.h"
+#include "xgene_enet_sgmac.h"
#include "xgene_enet_xgmac.h"
static void xgene_enet_init_bufpool(struct xgene_enet_desc_ring *buf_pool)
@@ -813,6 +814,7 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
return pdata->phy_mode;
}
if (pdata->phy_mode != PHY_INTERFACE_MODE_RGMII &&
+ pdata->phy_mode != PHY_INTERFACE_MODE_SGMII &&
pdata->phy_mode != PHY_INTERFACE_MODE_XGMII) {
dev_err(dev, "Incorrect phy-connection-type specified\n");
return -ENODEV;
@@ -830,7 +832,8 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
pdata->eth_csr_addr = base_addr + BLOCK_ETH_CSR_OFFSET;
pdata->eth_ring_if_addr = base_addr + BLOCK_ETH_RING_IF_OFFSET;
pdata->eth_diag_csr_addr = base_addr + BLOCK_ETH_DIAG_CSR_OFFSET;
- if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) {
+ if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII ||
+ pdata->phy_mode == PHY_INTERFACE_MODE_SGMII) {
pdata->mcx_mac_addr = base_addr + BLOCK_ETH_MAC_OFFSET;
pdata->mcx_mac_csr_addr = base_addr + BLOCK_ETH_MAC_CSR_OFFSET;
} else {
@@ -881,6 +884,11 @@ static void xgene_enet_setup_ops(struct xgene_enet_pdata *pdata)
pdata->port_ops = &xgene_gport_ops;
pdata->rm = RM3;
break;
+ case PHY_INTERFACE_MODE_SGMII:
+ pdata->mac_ops = &xgene_sgmac_ops;
+ pdata->port_ops = &xgene_sgport_ops;
+ pdata->rm = RM1;
+ break;
default:
pdata->mac_ops = &xgene_xgmac_ops;
pdata->port_ops = &xgene_xgport_ops;
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
index 10b03a1..874e5a0 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
@@ -39,6 +39,9 @@
#define NUM_PKT_BUF 64
#define NUM_BUFPOOL 32
+#define PHY_POLL_LINK_ON (10 * HZ)
+#define PHY_POLL_LINK_OFF (PHY_POLL_LINK_ON / 5)
+
/* software context of a descriptor ring */
struct xgene_enet_desc_ring {
struct net_device *ndev;
@@ -118,6 +121,13 @@ struct xgene_enet_pdata {
struct delayed_work link_work;
};
+struct xgene_indirect_ctl {
+ void __iomem *addr;
+ void __iomem *ctl;
+ void __iomem *cmd;
+ void __iomem *cmd_done;
+};
+
/* Set the specified value into a bit-field defined by its starting position
* and length within a single u64.
*/
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
new file mode 100644
index 0000000..e6d24c2
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
@@ -0,0 +1,389 @@
+/* Applied Micro X-Gene SoC Ethernet Driver
+ *
+ * Copyright (c) 2014, Applied Micro Circuits Corporation
+ * Authors: Iyappan Subramanian <isubramanian@apm.com>
+ * Keyur Chudgar <kchudgar@apm.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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "xgene_enet_main.h"
+#include "xgene_enet_hw.h"
+#include "xgene_enet_sgmac.h"
+
+static void xgene_enet_wr_csr(struct xgene_enet_pdata *p, u32 offset, u32 val)
+{
+ iowrite32(val, p->eth_csr_addr + offset);
+}
+
+static void xgene_enet_wr_ring_if(struct xgene_enet_pdata *p,
+ u32 offset, u32 val)
+{
+ iowrite32(val, p->eth_ring_if_addr + offset);
+}
+
+static void xgene_enet_wr_diag_csr(struct xgene_enet_pdata *p,
+ u32 offset, u32 val)
+{
+ iowrite32(val, p->eth_diag_csr_addr + offset);
+}
+
+static bool xgene_enet_wr_indirect(struct xgene_indirect_ctl *ctl,
+ u32 wr_addr, u32 wr_data)
+{
+ int i;
+
+ iowrite32(wr_addr, ctl->addr);
+ iowrite32(wr_data, ctl->ctl);
+ iowrite32(XGENE_ENET_WR_CMD, ctl->cmd);
+
+ /* wait for write command to complete */
+ for (i = 0; i < 10; i++) {
+ if (ioread32(ctl->cmd_done)) {
+ iowrite32(0, ctl->cmd);
+ return true;
+ }
+ udelay(1);
+ }
+
+ return false;
+}
+
+static void xgene_enet_wr_mac(struct xgene_enet_pdata *p,
+ u32 wr_addr, u32 wr_data)
+{
+ struct xgene_indirect_ctl ctl = {
+ .addr = p->mcx_mac_addr + MAC_ADDR_REG_OFFSET,
+ .ctl = p->mcx_mac_addr + MAC_WRITE_REG_OFFSET,
+ .cmd = p->mcx_mac_addr + MAC_COMMAND_REG_OFFSET,
+ .cmd_done = p->mcx_mac_addr + MAC_COMMAND_DONE_REG_OFFSET
+ };
+
+ if (!xgene_enet_wr_indirect(&ctl, wr_addr, wr_data))
+ netdev_err(p->ndev, "mac write failed, addr: %04x\n", wr_addr);
+}
+
+static u32 xgene_enet_rd_csr(struct xgene_enet_pdata *p, u32 offset)
+{
+ return ioread32(p->eth_csr_addr + offset);
+}
+
+static u32 xgene_enet_rd_diag_csr(struct xgene_enet_pdata *p, u32 offset)
+{
+ return ioread32(p->eth_diag_csr_addr + offset);
+}
+
+static u32 xgene_enet_rd_indirect(struct xgene_indirect_ctl *ctl, u32 rd_addr)
+{
+ u32 rd_data;
+ int i;
+
+ iowrite32(rd_addr, ctl->addr);
+ iowrite32(XGENE_ENET_RD_CMD, ctl->cmd);
+
+ /* wait for read command to complete */
+ for (i = 0; i < 10; i++) {
+ if (ioread32(ctl->cmd_done)) {
+ rd_data = ioread32(ctl->ctl);
+ iowrite32(0, ctl->cmd);
+
+ return rd_data;
+ }
+ udelay(1);
+ }
+
+ pr_err("%s: mac read failed, addr: %04x\n", __func__, rd_addr);
+
+ return 0;
+}
+
+static u32 xgene_enet_rd_mac(struct xgene_enet_pdata *p, u32 rd_addr)
+{
+ struct xgene_indirect_ctl ctl = {
+ .addr = p->mcx_mac_addr + MAC_ADDR_REG_OFFSET,
+ .ctl = p->mcx_mac_addr + MAC_READ_REG_OFFSET,
+ .cmd = p->mcx_mac_addr + MAC_COMMAND_REG_OFFSET,
+ .cmd_done = p->mcx_mac_addr + MAC_COMMAND_DONE_REG_OFFSET
+ };
+
+ return xgene_enet_rd_indirect(&ctl, rd_addr);
+}
+
+static int xgene_enet_ecc_init(struct xgene_enet_pdata *p)
+{
+ struct net_device *ndev = p->ndev;
+ u32 data;
+ int i;
+
+ xgene_enet_wr_diag_csr(p, ENET_CFG_MEM_RAM_SHUTDOWN_ADDR, 0);
+ for (i = 0; i < 10 && data != ~0U ; i++) {
+ usleep_range(100, 110);
+ data = xgene_enet_rd_diag_csr(p, ENET_BLOCK_MEM_RDY_ADDR);
+ }
+
+ if (data != ~0U) {
+ netdev_err(ndev, "Failed to release memory from shutdown\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static void xgene_enet_config_ring_if_assoc(struct xgene_enet_pdata *p)
+{
+ u32 val = 0xffffffff;
+
+ xgene_enet_wr_ring_if(p, ENET_CFGSSQMIWQASSOC_ADDR, val);
+ xgene_enet_wr_ring_if(p, ENET_CFGSSQMIFPQASSOC_ADDR, val);
+}
+
+static void xgene_mii_phy_write(struct xgene_enet_pdata *p, u8 phy_id,
+ u32 reg, u16 data)
+{
+ u32 addr, wr_data, done;
+ int i;
+
+ addr = PHY_ADDR(phy_id) | REG_ADDR(reg);
+ xgene_enet_wr_mac(p, MII_MGMT_ADDRESS_ADDR, addr);
+
+ wr_data = PHY_CONTROL(data);
+ xgene_enet_wr_mac(p, MII_MGMT_CONTROL_ADDR, wr_data);
+
+ for (i = 0; i < 10; i++) {
+ done = xgene_enet_rd_mac(p, MII_MGMT_INDICATORS_ADDR);
+ if (!(done & BUSY_MASK))
+ return;
+ usleep_range(10, 20);
+ }
+
+ netdev_err(p->ndev, "MII_MGMT write failed\n");
+}
+
+static u32 xgene_mii_phy_read(struct xgene_enet_pdata *p, u8 phy_id, u32 reg)
+{
+ u32 addr, data, done;
+ int i;
+
+ addr = PHY_ADDR(phy_id) | REG_ADDR(reg);
+ xgene_enet_wr_mac(p, MII_MGMT_ADDRESS_ADDR, addr);
+ xgene_enet_wr_mac(p, MII_MGMT_COMMAND_ADDR, READ_CYCLE_MASK);
+
+ for (i = 0; i < 10; i++) {
+ done = xgene_enet_rd_mac(p, MII_MGMT_INDICATORS_ADDR);
+ if (!(done & BUSY_MASK)) {
+ data = xgene_enet_rd_mac(p, MII_MGMT_STATUS_ADDR);
+ xgene_enet_wr_mac(p, MII_MGMT_COMMAND_ADDR, 0);
+
+ return data;
+ }
+ usleep_range(10, 20);
+ }
+
+ netdev_err(p->ndev, "MII_MGMT read failed\n");
+
+ return 0;
+}
+
+static void xgene_sgmac_reset(struct xgene_enet_pdata *p)
+{
+ xgene_enet_wr_mac(p, MAC_CONFIG_1_ADDR, SOFT_RESET1);
+ xgene_enet_wr_mac(p, MAC_CONFIG_1_ADDR, 0);
+}
+
+static void xgene_sgmac_set_mac_addr(struct xgene_enet_pdata *p)
+{
+ u32 addr0, addr1;
+ u8 *dev_addr = p->ndev->dev_addr;
+
+ addr0 = (dev_addr[3] << 24) | (dev_addr[2] << 16) |
+ (dev_addr[1] << 8) | dev_addr[0];
+ xgene_enet_wr_mac(p, STATION_ADDR0_ADDR, addr0);
+
+ addr1 = xgene_enet_rd_mac(p, STATION_ADDR1_ADDR);
+ addr1 |= (dev_addr[5] << 24) | (dev_addr[4] << 16);
+ xgene_enet_wr_mac(p, STATION_ADDR1_ADDR, addr1);
+}
+
+static u32 xgene_enet_link_status(struct xgene_enet_pdata *p)
+{
+ u32 data;
+
+ data = xgene_mii_phy_read(p, INT_PHY_ADDR,
+ SGMII_BASE_PAGE_ABILITY_ADDR >> 2);
+
+ return data & LINK_UP;
+}
+
+static void xgene_sgmac_init(struct xgene_enet_pdata *p)
+{
+ u32 data, loop = 10;
+
+ xgene_sgmac_reset(p);
+
+ /* Enable auto-negotiation */
+ xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_CONTROL_ADDR >> 2, 0x1000);
+ xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_TBI_CONTROL_ADDR >> 2, 0);
+
+ while (loop--) {
+ data = xgene_mii_phy_read(p, INT_PHY_ADDR,
+ SGMII_STATUS_ADDR >> 2);
+ if ((data & AUTO_NEG_COMPLETE) && (data & LINK_STATUS))
+ break;
+ usleep_range(10, 20);
+ }
+ if (!(data & AUTO_NEG_COMPLETE) || !(data & LINK_STATUS))
+ netdev_err(p->ndev, "Auto-negotiation failed\n");
+
+ data = xgene_enet_rd_mac(p, MAC_CONFIG_2_ADDR);
+ ENET_INTERFACE_MODE2_SET(&data, 2);
+ xgene_enet_wr_mac(p, MAC_CONFIG_2_ADDR, data | FULL_DUPLEX2);
+ xgene_enet_wr_mac(p, INTERFACE_CONTROL_ADDR, ENET_GHD_MODE);
+
+ data = xgene_enet_rd_csr(p, ENET_SPARE_CFG_REG_ADDR);
+ data |= MPA_IDLE_WITH_QMI_EMPTY;
+ xgene_enet_wr_csr(p, ENET_SPARE_CFG_REG_ADDR, data);
+
+ xgene_sgmac_set_mac_addr(p);
+
+ data = xgene_enet_rd_csr(p, DEBUG_REG_ADDR);
+ data |= CFG_BYPASS_UNISEC_TX | CFG_BYPASS_UNISEC_RX;
+ xgene_enet_wr_csr(p, DEBUG_REG_ADDR, data);
+
+ /* Adjust MDC clock frequency */
+ data = xgene_enet_rd_mac(p, MII_MGMT_CONFIG_ADDR);
+ MGMT_CLOCK_SEL_SET(&data, 7);
+ xgene_enet_wr_mac(p, MII_MGMT_CONFIG_ADDR, data);
+
+ /* Enable drop if bufpool not available */
+ data = xgene_enet_rd_csr(p, RSIF_CONFIG_REG_ADDR);
+ data |= CFG_RSIF_FPBUFF_TIMEOUT_EN;
+ xgene_enet_wr_csr(p, RSIF_CONFIG_REG_ADDR, data);
+
+ /* Rtype should be copied from FP */
+ xgene_enet_wr_csr(p, RSIF_RAM_DBG_REG0_ADDR, 0);
+
+ /* Bypass traffic gating */
+ xgene_enet_wr_csr(p, CFG_LINK_AGGR_RESUME_0_ADDR, TX_PORT0);
+ xgene_enet_wr_csr(p, CFG_BYPASS_ADDR, RESUME_TX);
+ xgene_enet_wr_csr(p, SG_RX_DV_GATE_REG_0_ADDR, RESUME_RX0);
+}
+
+static void xgene_sgmac_rxtx(struct xgene_enet_pdata *p, u32 bits, bool set)
+{
+ u32 data;
+
+ data = xgene_enet_rd_mac(p, MAC_CONFIG_1_ADDR);
+
+ if (set)
+ data |= bits;
+ else
+ data &= ~bits;
+
+ xgene_enet_wr_mac(p, MAC_CONFIG_1_ADDR, data);
+}
+
+static void xgene_sgmac_rx_enable(struct xgene_enet_pdata *p)
+{
+ xgene_sgmac_rxtx(p, RX_EN, true);
+}
+
+static void xgene_sgmac_tx_enable(struct xgene_enet_pdata *p)
+{
+ xgene_sgmac_rxtx(p, TX_EN, true);
+}
+
+static void xgene_sgmac_rx_disable(struct xgene_enet_pdata *p)
+{
+ xgene_sgmac_rxtx(p, RX_EN, false);
+}
+
+static void xgene_sgmac_tx_disable(struct xgene_enet_pdata *p)
+{
+ xgene_sgmac_rxtx(p, TX_EN, false);
+}
+
+static void xgene_enet_reset(struct xgene_enet_pdata *p)
+{
+ clk_prepare_enable(p->clk);
+ clk_disable_unprepare(p->clk);
+ clk_prepare_enable(p->clk);
+
+ xgene_enet_ecc_init(p);
+ xgene_enet_config_ring_if_assoc(p);
+}
+
+static void xgene_enet_cle_bypass(struct xgene_enet_pdata *p,
+ u32 dst_ring_num, u16 bufpool_id)
+{
+ u32 data, fpsel;
+
+ data = CFG_CLE_BYPASS_EN0;
+ xgene_enet_wr_csr(p, CLE_BYPASS_REG0_0_ADDR, data);
+
+ fpsel = xgene_enet_ring_bufnum(bufpool_id) - 0x20;
+ data = CFG_CLE_DSTQID0(dst_ring_num) | CFG_CLE_FPSEL0(fpsel);
+ xgene_enet_wr_csr(p, CLE_BYPASS_REG1_0_ADDR, data);
+}
+
+static void xgene_enet_shutdown(struct xgene_enet_pdata *p)
+{
+ clk_disable_unprepare(p->clk);
+}
+
+static void xgene_enet_link_state(struct work_struct *work)
+{
+ struct xgene_enet_pdata *p = container_of(to_delayed_work(work),
+ struct xgene_enet_pdata, link_work);
+ struct net_device *ndev = p->ndev;
+ u32 link, poll_interval;
+
+ link = xgene_enet_link_status(p);
+ if (link) {
+ if (!netif_carrier_ok(ndev)) {
+ netif_carrier_on(ndev);
+ xgene_sgmac_init(p);
+ xgene_sgmac_rx_enable(p);
+ xgene_sgmac_tx_enable(p);
+ netdev_info(ndev, "Link is Up - 1Gbps\n");
+ }
+ poll_interval = PHY_POLL_LINK_ON;
+ } else {
+ if (netif_carrier_ok(ndev)) {
+ xgene_sgmac_rx_disable(p);
+ xgene_sgmac_tx_disable(p);
+ netif_carrier_off(ndev);
+ netdev_info(ndev, "Link is Down\n");
+ }
+ poll_interval = PHY_POLL_LINK_OFF;
+ }
+
+ schedule_delayed_work(&p->link_work, poll_interval);
+}
+
+struct xgene_mac_ops xgene_sgmac_ops = {
+ .init = xgene_sgmac_init,
+ .reset = xgene_sgmac_reset,
+ .rx_enable = xgene_sgmac_rx_enable,
+ .tx_enable = xgene_sgmac_tx_enable,
+ .rx_disable = xgene_sgmac_rx_disable,
+ .tx_disable = xgene_sgmac_tx_disable,
+ .set_mac_addr = xgene_sgmac_set_mac_addr,
+ .link_state = xgene_enet_link_state
+};
+
+struct xgene_port_ops xgene_sgport_ops = {
+ .reset = xgene_enet_reset,
+ .cle_bypass = xgene_enet_cle_bypass,
+ .shutdown = xgene_enet_shutdown
+};
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
new file mode 100644
index 0000000..de43246
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
@@ -0,0 +1,41 @@
+/* Applied Micro X-Gene SoC Ethernet Driver
+ *
+ * Copyright (c) 2014, Applied Micro Circuits Corporation
+ * Authors: Iyappan Subramanian <isubramanian@apm.com>
+ * Keyur Chudgar <kchudgar@apm.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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __XGENE_ENET_SGMAC_H__
+#define __XGENE_ENET_SGMAC_H__
+
+#define PHY_ADDR(src) (((src)<<8) & GENMASK(12, 8))
+#define REG_ADDR(src) ((src) & GENMASK(4, 0))
+#define PHY_CONTROL(src) ((src) & GENMASK(15, 0))
+#define INT_PHY_ADDR 0x1e
+#define SGMII_TBI_CONTROL_ADDR 0x44
+#define SGMII_CONTROL_ADDR 0x00
+#define SGMII_STATUS_ADDR 0x04
+#define SGMII_BASE_PAGE_ABILITY_ADDR 0x14
+#define AUTO_NEG_COMPLETE BIT(5)
+#define LINK_STATUS BIT(2)
+#define LINK_UP BIT(15)
+#define MPA_IDLE_WITH_QMI_EMPTY BIT(12)
+#define SG_RX_DV_GATE_REG_0_ADDR 0x0dfc
+
+extern struct xgene_mac_ops xgene_sgmac_ops;
+extern struct xgene_port_ops xgene_sgport_ops;
+
+#endif /* __XGENE_ENET_SGMAC_H__ */
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h
index dcb2087..5a5296a 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h
@@ -47,9 +47,6 @@
#define XG_ENET_SPARE_CFG_REG_1_ADDR 0x0410
#define XGENET_RX_DV_GATE_REG_0_ADDR 0x0804
-#define PHY_POLL_LINK_ON (10 * HZ)
-#define PHY_POLL_LINK_OFF (PHY_POLL_LINK_ON / 5)
-
extern struct xgene_mac_ops xgene_xgmac_ops;
extern struct xgene_port_ops xgene_xgport_ops;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/4] drivers: net: xgene: Preparing for adding SGMII based 1GbE
From: Iyappan Subramanian @ 2014-10-14 0:05 UTC (permalink / raw)
To: davem, romieu, netdev, devicetree
Cc: linux-arm-kernel, patches, kchudgar, Iyappan Subramanian
In-Reply-To: <1413245135-2989-1-git-send-email-isubramanian@apm.com>
- Added link_state function pointer to the xgene__mac_ops structure
- Moved ring manager (pdata->rm) assignment to xgene_enet_setup_ops
- Removed unused variable (pdata->phy_addr) and macro (FULL_DUPLEX)
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 1 -
drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 1 -
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 8 +++++---
drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 2 +-
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c | 3 ++-
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h | 1 -
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index c8f3824..63ea194 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -410,7 +410,6 @@ static void xgene_gmac_set_mac_addr(struct xgene_enet_pdata *pdata)
addr0 = (dev_addr[3] << 24) | (dev_addr[2] << 16) |
(dev_addr[1] << 8) | dev_addr[0];
addr1 = (dev_addr[5] << 24) | (dev_addr[4] << 16);
- addr1 |= pdata->phy_addr & 0xFFFF;
xgene_enet_wr_mcx_mac(pdata, STATION_ADDR0_ADDR, addr0);
xgene_enet_wr_mcx_mac(pdata, STATION_ADDR1_ADDR, addr1);
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
index 15ec426..2efc4d9 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
@@ -179,7 +179,6 @@ enum xgene_enet_rm {
#define TUND_ADDR 0x4a
#define TSO_IPPROTO_TCP 1
-#define FULL_DUPLEX 2
#define USERINFO_POS 0
#define USERINFO_LEN 32
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index 9b85239..9e251ec 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -833,11 +833,9 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII) {
pdata->mcx_mac_addr = base_addr + BLOCK_ETH_MAC_OFFSET;
pdata->mcx_mac_csr_addr = base_addr + BLOCK_ETH_MAC_CSR_OFFSET;
- pdata->rm = RM3;
} else {
pdata->mcx_mac_addr = base_addr + BLOCK_AXG_MAC_OFFSET;
pdata->mcx_mac_csr_addr = base_addr + BLOCK_AXG_MAC_CSR_OFFSET;
- pdata->rm = RM0;
}
pdata->rx_buff_cnt = NUM_PKT_BUF;
@@ -881,10 +879,12 @@ static void xgene_enet_setup_ops(struct xgene_enet_pdata *pdata)
case PHY_INTERFACE_MODE_RGMII:
pdata->mac_ops = &xgene_gmac_ops;
pdata->port_ops = &xgene_gport_ops;
+ pdata->rm = RM3;
break;
default:
pdata->mac_ops = &xgene_xgmac_ops;
pdata->port_ops = &xgene_xgport_ops;
+ pdata->rm = RM0;
break;
}
}
@@ -895,6 +895,7 @@ static int xgene_enet_probe(struct platform_device *pdev)
struct xgene_enet_pdata *pdata;
struct device *dev = &pdev->dev;
struct napi_struct *napi;
+ struct xgene_mac_ops *mac_ops;
int ret;
ndev = alloc_etherdev(sizeof(struct xgene_enet_pdata));
@@ -937,10 +938,11 @@ static int xgene_enet_probe(struct platform_device *pdev)
napi = &pdata->rx_ring->napi;
netif_napi_add(ndev, napi, xgene_enet_napi, NAPI_POLL_WEIGHT);
+ mac_ops = pdata->mac_ops;
if (pdata->phy_mode == PHY_INTERFACE_MODE_RGMII)
ret = xgene_enet_mdio_config(pdata);
else
- INIT_DELAYED_WORK(&pdata->link_work, xgene_enet_link_state);
+ INIT_DELAYED_WORK(&pdata->link_work, mac_ops->link_state);
return ret;
err:
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
index 86cf68b..10b03a1 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
@@ -76,6 +76,7 @@ struct xgene_mac_ops {
void (*tx_disable)(struct xgene_enet_pdata *pdata);
void (*rx_disable)(struct xgene_enet_pdata *pdata);
void (*set_mac_addr)(struct xgene_enet_pdata *pdata);
+ void (*link_state)(struct work_struct *work);
};
struct xgene_port_ops {
@@ -109,7 +110,6 @@ struct xgene_enet_pdata {
void __iomem *base_addr;
void __iomem *ring_csr_addr;
void __iomem *ring_cmd_addr;
- u32 phy_addr;
int phy_mode;
enum xgene_enet_rm rm;
struct rtnl_link_stats64 stats;
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
index cd64b9f..67d0720 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
@@ -284,7 +284,7 @@ static void xgene_enet_shutdown(struct xgene_enet_pdata *pdata)
clk_disable_unprepare(pdata->clk);
}
-void xgene_enet_link_state(struct work_struct *work)
+static void xgene_enet_link_state(struct work_struct *work)
{
struct xgene_enet_pdata *pdata = container_of(to_delayed_work(work),
struct xgene_enet_pdata, link_work);
@@ -322,6 +322,7 @@ struct xgene_mac_ops xgene_xgmac_ops = {
.rx_disable = xgene_xgmac_rx_disable,
.tx_disable = xgene_xgmac_tx_disable,
.set_mac_addr = xgene_xgmac_set_mac_addr,
+ .link_state = xgene_enet_link_state
};
struct xgene_port_ops xgene_xgport_ops = {
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h
index d2d59e7..dcb2087 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h
@@ -50,7 +50,6 @@
#define PHY_POLL_LINK_ON (10 * HZ)
#define PHY_POLL_LINK_OFF (PHY_POLL_LINK_ON / 5)
-void xgene_enet_link_state(struct work_struct *work);
extern struct xgene_mac_ops xgene_xgmac_ops;
extern struct xgene_port_ops xgene_xgport_ops;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/4] dtb: Add SGMII based 1GbE node to APM X-Gene SoC device tree
From: Iyappan Subramanian @ 2014-10-14 0:05 UTC (permalink / raw)
To: davem, romieu, netdev, devicetree
Cc: linux-arm-kernel, patches, kchudgar, Iyappan Subramanian
In-Reply-To: <1413245135-2989-1-git-send-email-isubramanian@apm.com>
Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
arch/arm64/boot/dts/apm-mustang.dts | 4 ++++
arch/arm64/boot/dts/apm-storm.dtsi | 24 ++++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/arch/arm64/boot/dts/apm-mustang.dts b/arch/arm64/boot/dts/apm-mustang.dts
index 2ae782b..71a1489 100644
--- a/arch/arm64/boot/dts/apm-mustang.dts
+++ b/arch/arm64/boot/dts/apm-mustang.dts
@@ -33,6 +33,10 @@
status = "ok";
};
+&sgenet0 {
+ status = "ok";
+};
+
&xgenet {
status = "ok";
};
diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index d16cc03..f45bbfe 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -176,6 +176,16 @@
clock-output-names = "menetclk";
};
+ sge0clk: sge0clk@1f21c000 {
+ compatible = "apm,xgene-device-clock";
+ #clock-cells = <1>;
+ clocks = <&socplldiv2 0>;
+ reg = <0x0 0x1f21c000 0x0 0x1000>;
+ reg-names = "csr-reg";
+ csr-mask = <0x3>;
+ clock-output-names = "sge0clk";
+ };
+
xge0clk: xge0clk@1f61c000 {
compatible = "apm,xgene-device-clock";
#clock-cells = <1>;
@@ -446,6 +456,20 @@
};
};
+ sgenet0: ethernet@1f210000 {
+ compatible = "apm,xgene-enet";
+ status = "disabled";
+ reg = <0x0 0x1f210000 0x0 0x10000>,
+ <0x0 0x1f200000 0x0 0X10000>,
+ <0x0 0x1B000000 0x0 0X20000>;
+ reg-names = "enet_csr", "ring_csr", "ring_cmd";
+ interrupts = <0x0 0xA0 0x4>;
+ dma-coherent;
+ clocks = <&sge0clk 0>;
+ local-mac-address = [00 00 00 00 00 00];
+ phy-connection-type = "sgmii";
+ };
+
xgenet: ethernet@1f610000 {
compatible = "apm,xgene-enet";
status = "disabled";
--
1.9.1
^ permalink raw reply related
* [PATCH v2 0/4] Add SGMII based 1GbE support to APM X-Gene SoC ethernet driver
From: Iyappan Subramanian @ 2014-10-14 0:05 UTC (permalink / raw)
To: davem, romieu, netdev, devicetree
Cc: linux-arm-kernel, patches, kchudgar, Iyappan Subramanian
Adding SGMII based 1GbE basic support to APM X-Gene SoC ethernet driver.
v2: Address comments from v1
* Split the patchset into two, the first one being preparatory patch
* Added link_state function pointer to the xgene_mac_ops structure
* Added xgene_indirect_ctl structure for indirect read/write arguments
v1:
* Initial version
---
Iyappan Subramanian (4):
dtb: Add SGMII based 1GbE node to APM X-Gene SoC device tree
drivers: net: xgene: Preparing for adding SGMII based 1GbE
drivers: net: xgene: Add SGMII based 1GbE support
drivers: net: xgene: Add SGMII based 1GbE ethtool support
arch/arm64/boot/dts/apm-mustang.dts | 4 +
arch/arm64/boot/dts/apm-storm.dtsi | 24 ++
drivers/net/ethernet/apm/xgene/Makefile | 2 +-
.../net/ethernet/apm/xgene/xgene_enet_ethtool.c | 25 +-
drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 1 -
drivers/net/ethernet/apm/xgene/xgene_enet_hw.h | 4 +-
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 18 +-
drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 12 +-
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 389 +++++++++++++++++++++
drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h | 41 +++
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c | 3 +-
drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h | 4 -
12 files changed, 506 insertions(+), 21 deletions(-)
create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
--
1.9.1
^ permalink raw reply
* Re: something is wrong in commit 971f10eca1 - tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Cong Wang @ 2014-10-13 23:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Krzysztof Kolasa, netdev, edumazet, David Miller
In-Reply-To: <1413139613.9362.78.camel@edumazet-glaptop2.roam.corp.google.com>
Probably not related with this bug, but with regarding to the
offending commit, what's the point of the memmove() in tcp_v4_rcv()
since ip_rcv() already clears IPCB()?
^ permalink raw reply
* Re: [PATCH v1 2/3] drivers: net: xgene: Add SGMII based 1GbE support
From: Iyappan Subramanian @ 2014-10-13 23:19 UTC (permalink / raw)
To: Francois Romieu
Cc: David Miller, netdev, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, patches, Keyur Chudgar
In-Reply-To: <20141010220112.GA7633@electric-eye.fr.zoreil.com>
Thanks for the review.
On Fri, Oct 10, 2014 at 3:01 PM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Iyappan Subramanian <isubramanian@apm.com> :
> [...]
>> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
>> index c8f3824..63ea194 100644
>> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
>> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
>> @@ -410,7 +410,6 @@ static void xgene_gmac_set_mac_addr(struct xgene_enet_pdata *pdata)
>> addr0 = (dev_addr[3] << 24) | (dev_addr[2] << 16) |
>> (dev_addr[1] << 8) | dev_addr[0];
>> addr1 = (dev_addr[5] << 24) | (dev_addr[4] << 16);
>> - addr1 |= pdata->phy_addr & 0xFFFF;
>
> phy_addr removal is harmless (all zeroes from netdev priv data) but it's
> unrelated to $SUBJECT.
>
> You may split this patch as:
> 1. prettyfication / cruft removal
> 2. add link_state in xgene_mac_ops / pdata->rm shuffle
> 3. SGMII based 1GbE support
I will split the patch into two, the first one being preparatory patch.
>
> Mostly stylistic review below.
>
> [...]
>> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
>> index 15ec426..dc024c1 100644
>> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
>> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
> [...]
>> @@ -179,7 +180,6 @@ enum xgene_enet_rm {
>> #define TUND_ADDR 0x4a
>>
>> #define TSO_IPPROTO_TCP 1
>> -#define FULL_DUPLEX 2
>
> See above.
>
>>
>> #define USERINFO_POS 0
>> #define USERINFO_LEN 32
> [...]
>> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
>> new file mode 100644
>> index 0000000..6038596
>> --- /dev/null
>> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c
> [...]
>> +static void xgene_enet_wr_csr(struct xgene_enet_pdata *pdata,
>> + u32 offset, u32 val)
>> +{
>> + void __iomem *addr = pdata->eth_csr_addr + offset;
>> +
>> + iowrite32(val, addr);
>> +}
>
> Replace 'pdata' with one of 'xp', 'pd', 'p' ?
>
> You should be able to pack a lot.
That helps! I will use 'p' where ever possible and try to be consistent.
>
> static void xgene_enet_wr_csr(struct xgene_enet_pdata *p, u32 offset, u32 val)
> {
> iowrite32(val, p->eth_csr_addr + offset);
> }
>
> There are several of those.
>
> [...]
>> +static bool xgene_enet_wr_indirect(void __iomem *addr, void __iomem *wr,
>> + void __iomem *cmd, void __iomem *cmd_done,
>> + u32 wr_addr, u32 wr_data)
>> +{
>> + u32 done;
>> + u8 wait = 10;
>> +
>> + iowrite32(wr_addr, addr);
>> + iowrite32(wr_data, wr);
>> + iowrite32(XGENE_ENET_WR_CMD, cmd);
>> +
>> + /* wait for write command to complete */
>> + while (!(done = ioread32(cmd_done)) && wait--)
>> + udelay(1);
>> +
>> + if (!done)
>> + return false;
>
> int i;
>
> for (i = 0; i < 10; i++) {
> if (ioread32(cmd_done)) {
> iowrite32(0, cmd);
> return true;
> }
> udelay(1);
> }
>
> return false;
>
Sure. I will use the 'for' loop.
>> +
>> + iowrite32(0, cmd);
>> +
>> + return true;
>> +}
>> +
>> +static void xgene_enet_wr_mac(struct xgene_enet_pdata *pdata,
>> + u32 wr_addr, u32 wr_data)
>> +{
>> + void __iomem *addr, *wr, *cmd, *cmd_done;
>> +
>> + addr = pdata->mcx_mac_addr + MAC_ADDR_REG_OFFSET;
>> + wr = pdata->mcx_mac_addr + MAC_WRITE_REG_OFFSET;
>> + cmd = pdata->mcx_mac_addr + MAC_COMMAND_REG_OFFSET;
>> + cmd_done = pdata->mcx_mac_addr + MAC_COMMAND_DONE_REG_OFFSET;
>
> struct xgene_indirect_ctl {
> void __iomem *addr;
> void __iomem *ctl;
> void __iomem *cmd;
> void __iomem *cmd_done;
> };
>
> static void xgene_enet_wr_mac(struct xgene_enet_pdata *p, u32 addr, u32 data)
> {
> struct xgene_indirect_ctl ctl = {
> .addr = p->mcx_mac_addr + MAC_ADDR_REG_OFFSET;
> .ctl = p->mcx_mac_addr + MAC_WRITE_REG_OFFSET;
> .cmd = p->mcx_mac_addr + MAC_COMMAND_REG_OFFSET;
> .cmd_done = p->mcx_mac_addr + MAC_COMMAND_DONE_REG_OFFSET;
> };
>
> if (!xgene_enet_wr_indirect(&ctl, wr_addr, wr_data)) {
> ...
>
> It's syntaxic sugar that avoids (an excess of) 'void *' parameters.
I agree and will use xgene_indirect_ctl.
>
> You could reuse it for xgene_enet_rd_mac.
>
>> +
>> + if (!xgene_enet_wr_indirect(addr, wr, cmd, cmd_done, wr_addr, wr_data))
>> + netdev_err(pdata->ndev, "MCX mac write failed, addr: %04x\n",
>> + wr_addr);
>> +}
>> +
>> +static void xgene_enet_rd_csr(struct xgene_enet_pdata *pdata,
>> + u32 offset, u32 *val)
>> +{
>> + void __iomem *addr = pdata->eth_csr_addr + offset;
>> +
>> + *val = ioread32(addr);
>> +}
>
> static u32 xgene_enet_rd_csr(struct xgene_enet_pdata *pdata, u32 offset)
> {
> return ioread32(pdata->eth_csr_addr + offset);
> }
I will change as you suggested and it is consistent with ioread32().
>
>> +
>> +static void xgene_enet_rd_diag_csr(struct xgene_enet_pdata *pdata,
>> + u32 offset, u32 *val)
>> +{
>> + void __iomem *addr = pdata->eth_diag_csr_addr + offset;
>> +
>> + *val = ioread32(addr);
>> +}
>> +
>> +static bool xgene_enet_rd_indirect(void __iomem *addr, void __iomem *rd,
>> + void __iomem *cmd, void __iomem *cmd_done,
>> + u32 rd_addr, u32 *rd_data)
>> +{
>> + u32 done;
>> + u8 wait = 10;
>> +
>> + iowrite32(rd_addr, addr);
>> + iowrite32(XGENE_ENET_RD_CMD, cmd);
>> +
>> + /* wait for read command to complete */
>> + while (!(done = ioread32(cmd_done)) && wait--)
>> + udelay(1);
>> +
>> + if (!done)
>> + return false;
>
> See above.
>
> [...]
>> +static void xgene_sgmac_rx_enable(struct xgene_enet_pdata *pdata)
>> +{
>> + u32 data;
>> +
>> + xgene_enet_rd_mac(pdata, MAC_CONFIG_1_ADDR, &data);
>> + xgene_enet_wr_mac(pdata, MAC_CONFIG_1_ADDR, data | RX_EN);
>> +}
>> +
>> +static void xgene_sgmac_tx_enable(struct xgene_enet_pdata *pdata)
>> +{
>> + u32 data;
>> +
>> + xgene_enet_rd_mac(pdata, MAC_CONFIG_1_ADDR, &data);
>> + xgene_enet_wr_mac(pdata, MAC_CONFIG_1_ADDR, data | TX_EN);
>> +}
>
> static void _xgene_sgmac_rxtx(struct xgene_enet_pdata *p, bool set, u32 bits)
> {
> u32 data;
>
> xgene_enet_rd_mac(pdata, MAC_CONFIG_1_ADDR, &data);
>
> if (set)
> data |= bits;
> else
> data &= ~bits;
>
> xgene_enet_wr_mac(pdata, MAC_CONFIG_1_ADDR, data);
> }
>
> (or _xgene_sgmac_rxtx(struct xgene_enet_pdata *p, u32 set, u32 clear))
>
> static void xgene_sgmac_rxtx_set(struct xgene_enet_pdata *p, u32 bits)
> {
> _xgene_sgmac_rxtx(p, true, bits);
> }
>
> static void xgene_sgmac_rx_enable(struct xgene_enet_pdata *p)
> {
> xgene_sgmac_rxtx_set(p, RX_EN);
Thanks for the suggestion. I would prefer to call _xgene_sgmac_rxtx
directly from here.
> }
>
> static void xgene_sgmac_tx_enable(struct xgene_enet_pdata *p)
> {
> xgene_sgmac_rxtx_set(p, TX_EN);
> }
>
> static void xgene_sgmac_rxtx_clear(struct xgene_enet_pdata *p, u32 bits)
> {
> _xgene_sgmac_rxtx(p, false, bits);
> }
>
> etc.
>
> [...]
>> +struct xgene_mac_ops xgene_sgmac_ops = {
>> + .init = xgene_sgmac_init,
>> + .reset = xgene_sgmac_reset,
>> + .rx_enable = xgene_sgmac_rx_enable,
>> + .tx_enable = xgene_sgmac_tx_enable,
>> + .rx_disable = xgene_sgmac_rx_disable,
>> + .tx_disable = xgene_sgmac_tx_disable,
>> + .set_mac_addr = xgene_sgmac_set_mac_addr,
>> + .link_state = xgene_enet_link_state
>
> Please use tabs before '='.
I will use tabs before "=" for xgene_mac_ops and xgene_port_ops
structure initialization.
>
>> +};
>> +
>> +struct xgene_port_ops xgene_sgport_ops = {
>> + .reset = xgene_enet_reset,
>> + .cle_bypass = xgene_enet_cle_bypass,
>> + .shutdown = xgene_enet_shutdown,
>
> See above.
>
> [...]
>> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
>> new file mode 100644
>> index 0000000..de43246
>> --- /dev/null
>> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
> [...]
>> +#define PHY_ADDR(src) (((src)<<8) & GENMASK(12, 8))
>
> #define PHY_ADDR(src) (((src) << 8) & GENMASK(12, 8))
>
> --
> Ueimor
^ permalink raw reply
* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-10-13 21:49 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <543C0A2D.5040908@redhat.com>
On Mon, Oct 13, 2014 at 10:21 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/03/2014 05:46 PM, Daniel Borkmann wrote:
> ...
>>
>> Ok, given you post the remaining two RFCs later on this window as
>> you indicate, I have no objections:
>>
>> Acked-by: Daniel Borkmann <dborkman@redhat.com>
>
>
> Ping, Alexei, are you still sending the patch for bpf_common.h or
> do you want me to take care of this?
It's not forgotten.
I'm not sending it only because net-next is closed
and it seems to be -next material.
^ permalink raw reply
* Re: Fw: [Bug 86081] New: Can't free the return value of sock_kmalloc() when the value is NULL
From: Cong Wang @ 2014-10-13 20:58 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Andy Grover, netdev
In-Reply-To: <20141012170756.19d1d12e@uryu.home.lan>
On Sun, Oct 12, 2014 at 8:07 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> in function rds_cmsg_rdma_args() at net/rds/rdma.c:L546, the variable
> "iovstack" is an array and the pointer variable *iovs is equal to iovstack (at
> Line 554). As the the return value of sock_kmalloc() (called at line 578),when
> "iovs" is NULL, function sock_kfree_s() will be called(at line 697) and
> function sock_kfree_s() will free "iovs".
> The related code snippets in function rds_cmsg_rdma_args() are as followings.
> rds_cmsg_rdma_args() at net/rds/rdma.c:L546
Free'ing NULL is okay, but not sub'ing the sk_omem_alloc size.
I will send a patch.
^ permalink raw reply
* Re: vxlan gro problem ?
From: Or Gerlitz @ 2014-10-13 20:56 UTC (permalink / raw)
To: yinpeijun
Cc: Or Gerlitz, qinchuanyu, Linux Netdev List, Linux Kernel, lichunhe,
wangfakai, liuyongan
In-Reply-To: <543B97F0.6040603@huawei.com>
On Mon, Oct 13, 2014 at 11:14 AM, yinpeijun <yinpeijun@huawei.com> wrote:
> On 2014/10/13 3:50, Or Gerlitz wrote:
> my test environment use mellanox ConnectX-3 Pro nic , as I know the nic support Rx checksum offload. but I am not confirm if should I do some special configure?
> or the nic driver or firmware need update ? also , I have used redhat7.0 ovs vxlan to test with the similar configure as before, but there is also no improvement .
The NIC (HW model and firmware) look just fine. As it seems now, this
boils down to get the RHEL7 inbox mlx4 driver to work properly on your
setup, something which goes a bit beyond the interest of the upstream
mailing lists...
Or.
>
> the nic infomation:
>
> 04:00.0 Ethernet controller: Mellanox Technologies MT27520 Family [ConnectX-3 Pro]
>
> root@localhost:~# ethtool -i eth4
> driver: mlx4_en
> version: 2.0(Dec 2011)
> firmware-version: 2.31.5050
> bus-info: 0000:04:00.0
> supports-statistics: yes
> supports-test: yes
> supports-eeprom-access: no
> supports-register-dump: no
> supports-priv-flags: yes
>
>
>
> --
> 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: Network optimality (was Re: [PATCH net-next] qdisc: validate skb without holding lock_
From: Dave Taht @ 2014-10-13 20:47 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, Alexander Duyck, Hannes Frederic Sowa,
John Fastabend, Jamal Hadi Salim, Daniel Borkmann,
Florian Westphal, netdev@vger.kernel.org,
Toke Høiland-Jørgensen, Tom Herbert, David Miller
In-Reply-To: <20141013222717.42f3c399@redhat.com>
On Mon, Oct 13, 2014 at 1:27 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Mon, 13 Oct 2014 10:20:17 -0700 Dave Taht <dave.taht@gmail.com> wrote:
>
>> On Mon, Oct 13, 2014 at 9:58 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> >
>> >> On Oct 13, 2014 7:22 AM, "Dave Taht" <dave.taht@gmail.com> wrote:
> [...]
>>
>> I would like to also get better behavior out of gigE and below, and for
>> these changes to not impact the downstream behavior of the network
>> overall.
>
> I also care about 1Gbit/s and below, that why I did some many tests
> (with igb at 10Mbit/s, 100Mbit/s and 1Gbit/s).
>
>
>> To give you an example, I would like to see the tcp flows in the
>> 2nd chart here, to converge faster than the 5 seconds they currently
>> take at GigE speeds.
>>
>> http://snapon.lab.bufferbloat.net/~cero2/nuc-to-puck/results.html
>
> In the last graph, where you cannot saturate the link, because you
> turned off GSO, GRO and TSO. Here I expect you will see the benefit of
> the qdisc bulking. That is, will be able to saturate the link and
> achieve the lower latency as BQL will cut off the bursts at +1 MTU.
> I would be interested in the results...
I am too!!!! 5 seconds to converge? 50x the baseline latency when under load?
vs not being able to saturate the link at all? Ugh. Two lousy choices.
I think xmit_more will
help a lot in the latter case, my other suggestions regarding reducing
the size of the offloads in the former.
But it looks like xmit_more support needs to be added to fq, and fq_codel (?),
and despite me reading the patches submitted thus far, it would be saner
for someone else to patch e1000e support for the nuc (and the zillions
of other e1000e platforms)
(did I miss that patch go by?)
I'm certainly willing to test the result on that platform (and I have
some other tweaks
in my queue at the qdisc layer that I can throw in, also).
>> > We made all these changes so that we can spend cpu cycles at the right
>> > place.
>
> Exactly.
+1.
So what happens when more cpu cycles are available in the right place?
The dequeue routines in both fq and fq_codel are a bit more complex than
pfifo_fast (and I've longed to kill the maxpacket concept in codel, btw)....
y'all are in such a lovely place with profilers and hardware at the ready
to just make a simple sysctl and analyze what happens at rates I can
only dream of.
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Sr. Network Kernel Developer at Red Hat
> Author of http://www.iptv-analyzer.org
> LinkedIn: http://www.linkedin.com/in/brouer
--
Dave Täht
https://www.bufferbloat.net/projects/make-wifi-fast
^ permalink raw reply
* Re: [ovs-dev] [PATCH/RFC repost 7/8] ofproto: translate datapath select group action
From: Ben Pfaff @ 2014-10-13 20:46 UTC (permalink / raw)
To: Simon Horman; +Cc: dev, netdev
In-Reply-To: <20141009011434.GE9339@vergenet.net>
On Thu, Oct 09, 2014 at 10:14:36AM +0900, Simon Horman wrote:
> On Fri, Sep 26, 2014 at 04:57:25PM -0700, Ben Pfaff wrote:
> > On Thu, Sep 18, 2014 at 10:55:10AM +0900, Simon Horman wrote:
> > > This patch is a prototype and has several limitations:
> > >
> > > * It assumes that no actions follow a select group action
> > > because the resulting packet after a select group action may
> > > differ depending on the bucket used. It may be possible
> > > to address this problem using recirculation. Or to not use
> > > the datapath select group in such situations. In any case
> > > this patch does not solve this problem or even prevent it
> > > from occurring.
> >
> > It seems like this limitation in particular is a pretty big one. Do
> > you have a good plan in mind for how to resolve it?
>
> Hi Ben,
>
> it seems to me that this would be somewhat difficult to resolve in the
> datapath so I propose not doing so. And I have two ideas on how to
> resolve this problem outside of the datapath.
>
> 1. Recirculation
>
> It seems to me that it ought to be possible to handle this by
> recirculating if actions occur after an ODP select group action.
>
> This could be made slightly more selective by only recirculating
> if the execution different buckets may result in different packet
> contents and the actions after the ODP select group action rely on
> the packet contents (e.g. set actions do but output actions do not).
>
> My feeling is that this could be implemented by adding a small amount
> of extra state to action translation in ovs-vswitchd.
>
> 2. Fall back to selecting buckets in ovs-vswtichd
>
> The idea here is to detect cases where there would be a problem
> executing actions after an ODP select group action and in that
> case to select buckets in ovs-vswtichd: that is use the existing bucket
> translation code in ovs-vswtichd.
>
> Though this seems conceptually simpler than recirculation it
> seems to me that it would be somewhat more difficult to implement
> as it implies a two stage translation process: e.g. one stage to
> determine if an ODP select group may be used; and one to perform
> the translation.
>
> I seem to recall trying various two stage translation processes
> as part some earlier unrelated work. And my recollection is that
> the result of my previous efforts were not pretty.
>
> Both of the above more or less negate any benefits of ODP select group
> action. In particular lowering flow setup cost and potentially allowing
> complete offload of select groups from the datapath to hardware. However I
> think that this case is not a common one as it requires both of the
> following. And I think they are both not usual use cases.
>
> * Different buckets modifying packets in different ways
> - My expectation is that it is common for buckets to be homogeneous in
> regards to packet modifications. But perhaps this is na??ve in the
> context of VLANs, MPLS, and similar tags that can be pushed and popped.
> * Actions that rely on packet contents after
> - My expectation is that it is common to use a select group to output
> packets and that is the final action performed.
I am glad that you have thought about it. Your ideas seem like a good
start to me. Personally, approach #2, of falling back to selecting
buckets in ovs-vswitchd, seems like a clean solution to me, although
if it really takes multiple stages in translation then that is
undesirable, so I hope that some clean and simple approach works out.
I think that we probably need a solution before we can apply the patch
series, because otherwise we end up with half-working code.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] caif_usb: remove redundant memory message
From: Joe Perches @ 2014-10-13 20:34 UTC (permalink / raw)
To: Fabian Frederick; +Cc: linux-kernel, Dmitry Tarnyagin, David S. Miller, netdev
In-Reply-To: <1413231946-9914-1-git-send-email-fabf@skynet.be>
On Mon, 2014-10-13 at 22:25 +0200, Fabian Frederick wrote:
> Let MM subsystem display out of memory messages.
[]
> diff --git a/net/caif/caif_usb.c b/net/caif/caif_usb.c
[]
> @@ -87,10 +87,9 @@ static struct cflayer *cfusbl_create(int phyid, u8 ethaddr[ETH_ALEN],
> {
> struct cfusbl *this = kmalloc(sizeof(struct cfusbl), GFP_ATOMIC);
>
> - if (!this) {
> - pr_warn("Out of memory\n");
> + if (!this)
> return NULL;
> - }
> +
> caif_assert(offsetof(struct cfusbl, layer) == 0);
>
> memset(this, 0, sizeof(struct cflayer));
This bit should probably be:
memset(&this->layer, 0, sizeof(this->layer));
or the allocation above should use kzalloc.
^ permalink raw reply
* Re: Network optimality (was Re: [PATCH net-next] qdisc: validate skb without holding lock_
From: Jesper Dangaard Brouer @ 2014-10-13 20:27 UTC (permalink / raw)
To: Dave Taht
Cc: Eric Dumazet, Alexander Duyck, Hannes Frederic Sowa,
John Fastabend, Jamal Hadi Salim, Daniel Borkmann,
Florian Westphal, netdev@vger.kernel.org,
Toke Høiland-Jørgensen, Tom Herbert, David Miller,
brouer
In-Reply-To: <CAA93jw5MKVaNaObVD3Lm=DP0ie6HcjwOvt0X-whDLJD_6hF5bw@mail.gmail.com>
On Mon, 13 Oct 2014 10:20:17 -0700 Dave Taht <dave.taht@gmail.com> wrote:
> On Mon, Oct 13, 2014 at 9:58 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >> On Oct 13, 2014 7:22 AM, "Dave Taht" <dave.taht@gmail.com> wrote:
[...]
>
> I would like to also get better behavior out of gigE and below, and for
> these changes to not impact the downstream behavior of the network
> overall.
I also care about 1Gbit/s and below, that why I did some many tests
(with igb at 10Mbit/s, 100Mbit/s and 1Gbit/s).
> To give you an example, I would like to see the tcp flows in the
> 2nd chart here, to converge faster than the 5 seconds they currently
> take at GigE speeds.
>
> http://snapon.lab.bufferbloat.net/~cero2/nuc-to-puck/results.html
In the last graph, where you cannot saturate the link, because you
turned off GSO, GRO and TSO. Here I expect you will see the benefit of
the qdisc bulking. That is, will be able to saturate the link and
achieve the lower latency as BQL will cut off the bursts at +1 MTU.
I would be interested in the results...
> > We made all these changes so that we can spend cpu cycles at the right
> > place.
Exactly.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH 1/1 net-next] caif_usb: remove redundant memory message
From: Fabian Frederick @ 2014-10-13 20:25 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, Dmitry Tarnyagin, David S. Miller, netdev
Let MM subsystem display out of memory messages.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/caif/caif_usb.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/caif/caif_usb.c b/net/caif/caif_usb.c
index ba02db0..0e487b0 100644
--- a/net/caif/caif_usb.c
+++ b/net/caif/caif_usb.c
@@ -87,10 +87,9 @@ static struct cflayer *cfusbl_create(int phyid, u8 ethaddr[ETH_ALEN],
{
struct cfusbl *this = kmalloc(sizeof(struct cfusbl), GFP_ATOMIC);
- if (!this) {
- pr_warn("Out of memory\n");
+ if (!this)
return NULL;
- }
+
caif_assert(offsetof(struct cfusbl, layer) == 0);
memset(this, 0, sizeof(struct cflayer));
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] caif: replace kmalloc/memset 0 by kzalloc
From: Fabian Frederick @ 2014-10-13 20:21 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, Dmitry Tarnyagin, David S. Miller, netdev
Also add blank line after declaration
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/caif/cfmuxl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/caif/cfmuxl.c b/net/caif/cfmuxl.c
index 8c5d638..510aa5a 100644
--- a/net/caif/cfmuxl.c
+++ b/net/caif/cfmuxl.c
@@ -47,10 +47,10 @@ static struct cflayer *get_up(struct cfmuxl *muxl, u16 id);
struct cflayer *cfmuxl_create(void)
{
- struct cfmuxl *this = kmalloc(sizeof(struct cfmuxl), GFP_ATOMIC);
+ struct cfmuxl *this = kzalloc(sizeof(struct cfmuxl), GFP_ATOMIC);
+
if (!this)
return NULL;
- memset(this, 0, sizeof(*this));
this->layer.receive = cfmuxl_receive;
this->layer.transmit = cfmuxl_transmit;
this->layer.ctrlcmd = cfmuxl_ctrlcmd;
--
1.9.3
^ permalink raw reply related
* Bug#763325: NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
From: ael @ 2014-10-13 20:20 UTC (permalink / raw)
To: netdev; +Cc: 763325
It was suggested that I post this bug here.
>From /var/log/messages:
=======================================================================
Oct 6 14:16:29 shelf kernel: [ 961.633769] ------------[ cut here ]------------
Oct 6 14:16:29 shelf kernel: [ 961.633790] WARNING: CPU: 0 PID: 0 at /build/linux-P15SNz/linux-3.16.3/net/sched/sch_generic.c:264
dev_watchdog+0x236/0x240()
Oct 6 14:16:29 shelf kernel: [ 961.633792] NETDEV WATCHDOG: eth0
(r8169): transmit queue 0 timed out
Oct 6 14:16:29 shelf kernel: [ 961.633794] Modules linked in:
sha256_ssse3 sha256_generic dm_crypt dm_mod usb_storage uinput cpuid
snd_hrtimer binfmt_misc snd_seq snd_seq_device bnep joydev nfsd
auth_rpcgss oid_registry nfs_acl nfs lockd fscache sunrpc
snd_hda_codec_realtek iTCO_wdt snd_hda_codec_hdmi snd_hda_codec_generic
iTCO_vendor_support ecb uvcvideo videobuf2_vmalloc btusb
videobuf2_memops videobuf2_core v4l2_common bluetooth videodev media
6lowpan_iphc arc4 rtsx_pci_sdmmc rtsx_pci_ms iwlmvm memstick mmc_core
mac80211 iwlwifi cfg80211 r8169 rtsx_pci mii rfkill x86_pkg_temp_thermal
intel_powerclamp intel_rapl coretemp kvm_intel kvm crc32_pclmul
crc32c_intel ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul
glue_helper ablk_helper cryptd lpc_ich evdev psmouse snd_hda_intel
serio_raw sr_mod snd_hda_controller pcspkr sg snd_hda_codec cdrom
mfd_core tpm_infineon i915 snd_hwdep i2c_i801 wmi shpchp snd_pcm
ehci_pci snd_timer tpm_tis drm_kms_helper ehci_hcd xhci_hcd snd drm
soundcore tpm i2c_algo_bit mei_me usbcore i2c_core mei video usb_common
ac battery processor button fuse parport_pc ppdev lp parport autofs4
ext4 crc16 mbcache jbd2 sd_mod crc_t10dif crct10dif_generic ahci libahci
crct10dif_pclmul crct10dif_common libata scsi_mod thermal thermal_sys
Oct 6 14:16:29 shelf kernel: [ 961.633898] CPU: 0 PID: 0 Comm:
swapper/0 Not tainted 3.16-2-amd64 #1 Debian 3.16.3-2
Oct 6 14:16:29 shelf kernel: [ 961.633899] Hardware name:
Notebook W54_55SU1,SUW/W54_55SU1,SUW, BIOS 4.6.5
05/29/2014
Oct 6 14:16:29 shelf kernel: [ 961.633901] 0000000000000009 ffffffff81506188 ffff88041fa03e28 ffffffff81065707
Oct 6 14:16:29 shelf kernel: [ 961.633903] 0000000000000000
ffff88041fa03e78 0000000000000001 0000000000000000
Oct 6 14:16:29 shelf kernel: [ 961.633907] ffff88040a0ec000
ffffffff8106576c ffffffff81775ca0 0000000000000030
Oct 6 14:16:29 shelf kernel: [ 961.633912] Call Trace:
Oct 6 14:16:29 shelf kernel: [ 961.633914] <IRQ>
[<ffffffff81506188>] ? dump_stack+0x41/0x51
Oct 6 14:16:29 shelf kernel: [ 961.633922] [<ffffffff81065707>] ?
warn_slowpath_common+0x77/0x90
Oct 6 14:16:29 shelf kernel: [ 961.633926] [<ffffffff8106576c>] ?
warn_slowpath_fmt+0x4c/0x50
Oct 6 14:16:29 shelf kernel: [ 961.633930] [<ffffffff81439af6>] ?
dev_watchdog+0x236/0x240
Oct 6 14:16:29 shelf kernel: [ 961.633936] [<ffffffff814398c0>] ?
dev_graft_qdisc+0x70/0x70
Oct 6 14:16:29 shelf kernel: [ 961.633941] [<ffffffff810709c1>] ?
call_timer_fn+0x31/0x100
Oct 6 14:16:29 shelf kernel: [ 961.633945] [<ffffffff814398c0>] ?
dev_graft_qdisc+0x70/0x70
Oct 6 14:16:29 shelf kernel: [ 961.633948] [<ffffffff81071ff9>] ?
run_timer_softirq+0x209/0x2f0
Oct 6 14:16:29 shelf kernel: [ 961.633953] [<ffffffff8106a5a1>] ?
__do_softirq+0xf1/0x290
Oct 6 14:16:29 shelf kernel: [ 961.633956] [<ffffffff8106a975>] ?
irq_exit+0x95/0xa0
Oct 6 14:16:29 shelf kernel: [ 961.633961] [<ffffffff8150f155>] ?
smp_apic_timer_interrupt+0x45/0x60
Oct 6 14:16:29 shelf kernel: [ 961.633965] [<ffffffff8150d25d>] ?
apic_timer_interrupt+0x6d/0x80
Oct 6 14:16:29 shelf kernel: [ 961.633968] <EOI>
[<ffffffff81088b5d>] ? __hrtimer_start_range_ns+0x1cd/0x390
Oct 6 14:16:29 shelf kernel: [ 961.633976] [<ffffffff813d96e2>] ?
cpuidle_enter_state+0x52/0xc0
Oct 6 14:16:29 shelf kernel: [ 961.633980] [<ffffffff813d96d8>] ?
cpuidle_enter_state+0x48/0xc0
Oct 6 14:16:29 shelf kernel: [ 961.633984] [<ffffffff810a5d28>] ?
cpu_startup_entry+0x2f8/0x400
Oct 6 14:16:29 shelf kernel: [ 961.633989] [<ffffffff8190305a>] ?
start_kernel+0x47b/0x486
Oct 6 14:16:29 shelf kernel: [ 961.633993] [<ffffffff81902a04>] ?
set_init_arg+0x4e/0x4e
Oct 6 14:16:29 shelf kernel: [ 961.633997] [<ffffffff81902120>] ?
early_idt_handlers+0x120/0x120
Oct 6 14:16:29 shelf kernel: [ 961.634000] [<ffffffff8190271f>] ?
x86_64_start_kernel+0x14d/0x15c
Oct 6 14:16:29 shelf kernel: [ 961.634004] ---[ end trace
065e688ac03d9c53 ]---
Oct 6 14:16:29 shelf kernel: [ 961.657322] r8169 0000:03:00.1 eth0:
link up
===================================================================
I believe I was changing from ethernet to wireless frequently while
doing some testing just before this particular instance. However this
bug is typicaly trigggered several times when just using eth0 and has
been happening for the last week.
For completeness, here are extracts from the log before the instance
above (when I was probably explicitly taking eth0 up and down, so it
possibly has irrelevant noise from those activities):
Oct 6 13:59:06 shelf kernel: [ 1.466463] r8169 0000:03:00.1 eth0:
RTL8411 at 0xffffc900018c4000, 80:fa:5b:04:ca:96, XID 1c800880 IRQ 44
Oct 6 13:59:06 shelf kernel: [ 1.466470] r8169 0000:03:00.1 eth0:
jumbo features [frames: 9200 bytes, tx checksumming: ko]
--[snip]--
Oct 6 13:59:06 shelf kernel: [ 1.556245] r8169 0000:03:00.1:
firmware: direct-loading firmware rtl_nic/rtl8411-2.fw
Oct 6 13:59:06 shelf kernel: [ 1.571948] r8169 0000:03:00.1 eth0:
link down
--[snip]--
Oct 6 13:59:09 shelf kernel: [ 5.463398] r8169 0000:03:00.1 eth0:
link up
--[snip]--
Oct 6 14:09:17 shelf kernel: [ 545.893079] r8169 0000:03:00.1 eth0:
link down
--[snip]--
Oct 6 14:09:17 shelf kernel: [ 547.647632] r8169 0000:03:00.1: no
hotplug settings from platform
Oct 6 14:09:17 shelf kernel: [ 547.648327] done.
Oct 6 14:09:17 shelf kernel: [ 547.660691] Bluetooth: hci0: read Intel
version: 370710018002030d37
Oct 6 14:09:17 shelf kernel: [ 547.660693] Bluetooth: hci0: Intel
device is already patched. patch num: 37
Oct 6 14:09:17 shelf kernel: [ 547.805664] [drm] Enabling RC6 states:
RC6 on, RC6p off, RC6pp off
Oct 6 14:09:23 shelf kernel: [ 553.115079] r8169 0000:03:00.1 eth0:
link up
Oct 6 14:09:24 shelf kernel: [ 554.134710] r8169 0000:03:00.1 eth0:
link down
Oct 6 14:09:27 shelf kernel: [ 557.556657] r8169 0000:03:00.1 eth0:
link up
Oct 6 14:09:28 shelf kernel: [ 558.578349] r8169 0000:03:00.1 eth0:
link down
Oct 6 14:09:32 shelf kernel: [ 562.018553] r8169 0000:03:00.1 eth0:
link up
Oct 6 14:09:33 shelf kernel: [ 563.040819] r8169 0000:03:00.1 eth0:
link down
Oct 6 14:09:36 shelf kernel: [ 566.548111] r8169 0000:03:00.1 eth0:
link up
Oct 6 14:09:37 shelf kernel: [ 567.568219] r8169 0000:03:00.1 eth0:
link down
Oct 6 14:09:41 shelf kernel: [ 571.786079] r8169 0000:03:00.1 eth0:
link up
Oct 6 14:09:42 shelf kernel: [ 572.808291] r8169 0000:03:00.1 eth0:
link down
Oct 6 14:09:45 shelf kernel: [ 575.383331] r8169 0000:03:00.1 eth0:
link up
--[snip]--
Oct 6 14:11:59 shelf kernel: [ 691.106397] r8169 0000:03:00.1 eth0:
link down
--[snip]--
Oct 6 14:11:59 shelf kernel: [ 691.974414] r8169 0000:03:00.1: no
hotplug settings from platform
--[snip]--
Oct 6 14:12:05 shelf kernel: [ 697.522170] r8169 0000:03:00.1 eth0:
link up
========================================================
lspci:
-----
00:00.0 Host bridge: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor DRAM Controller (rev 06)
00:02.0 VGA compatible controller: Intel Corporation 4th Gen Core Processor Integrated Graphics Controller (rev 06)
00:03.0 Audio device: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor HD Audio Controller (rev 06)
00:14.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB xHCI (rev 05)
00:16.0 Communication controller: Intel Corporation 8 Series/C220 Series Chipset Family MEI Controller #1 (rev 04)
00:1a.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB EHCI #2 (rev 05)
00:1b.0 Audio device: Intel Corporation 8 Series/C220 Series Chipset High Definition Audio Controller (rev 05)
00:1c.0 PCI bridge: Intel Corporation 8 Series/C220 Series Chipset Family PCI Express Root Port #1 (rev d5)
00:1c.2 PCI bridge: Intel Corporation 8 Series/C220 Series Chipset Family PCI Express Root Port #3 (rev d5)
00:1c.3 PCI bridge: Intel Corporation 8 Series/C220 Series Chipset Family PCI Express Root Port #4 (rev d5)
00:1d.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB EHCI #1 (rev 05)
00:1f.0 ISA bridge: Intel Corporation HM86 Express LPC Controller (rev 05)
00:1f.2 SATA controller: Intel Corporation 8 Series/C220 Series Chipset Family 6-port SATA Controller 1 [AHCI mode] (rev 05)
00:1f.3 SMBus: Intel Corporation 8 Series/C220 Series Chipset Family SMBus Controller (rev 05)
02:00.0 Network controller: Intel Corporation Wireless 7260 (rev 73)
03:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. Device 5287 (rev 01)
03:00.1 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 12)
-----------------------------------------------------------
lspci -v -s 03:00.1:
-------------------
03:00.1 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 12)
Subsystem: CLEVO/KAPOK Computer Device 5455
Flags: bus master, fast devsel, latency 0, IRQ 43
I/O ports at e000 [size=256]
Memory at f7c14000 (64-bit, non-prefetchable) [size=4K]
Memory at f7c10000 (64-bit, non-prefetchable) [size=16K]
Capabilities: <access denied>
Kernel driver in use: r8169
--------------------------------------------------------------------
I have not set the debug parameter on r8169 but can do so if that would
be useful.
ael
^ permalink raw reply
* Re: [PATCH] net: wireless: brcm80211: brcmfmac: dhd_sdio.c: Cleaning up missing null-terminate in conjunction with strncpy
From: Rickard Strandqvist @ 2014-10-13 20:14 UTC (permalink / raw)
To: David Laight
Cc: Brett Rudley, Arend van Spriel, Hante Meuleman, John W. Linville,
Pieter-Paul Giesberts, Daniel Kim, linux-wireless@vger.kernel.org,
brcm80211-dev-list@broadcom.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174C992F@AcuExch.aculab.com>
2014-10-13 10:55 GMT+02:00 David Laight <David.Laight@aculab.com>:
> From: Rickard Strandqvist
>> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
>> And changed from using strncpy to strlcpy to simplify code.
>
> I think you should return an error if the strings get truncated.
> Silent truncation is going to lead to issues at some point in the future
> (in some places).
>
>> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
>> ---
>> drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c | 25 ++++++++++----------
>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> index f55f625..d20d4e6 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> @@ -670,7 +670,6 @@ static int brcmf_sdio_get_fwnames(struct brcmf_chip *ci,
>> struct brcmf_sdio_dev *sdiodev)
>> {
>> int i;
>> - uint fw_len, nv_len;
>> char end;
>>
>> for (i = 0; i < ARRAY_SIZE(brcmf_fwname_data); i++) {
>> @@ -684,25 +683,25 @@ static int brcmf_sdio_get_fwnames(struct brcmf_chip *ci,
>> return -ENODEV;
>> }
>>
>> - fw_len = sizeof(sdiodev->fw_name) - 1;
>> - nv_len = sizeof(sdiodev->nvram_name) - 1;
>> /* check if firmware path is provided by module parameter */
>> if (brcmf_firmware_path[0] != '\0') {
>> - strncpy(sdiodev->fw_name, brcmf_firmware_path, fw_len);
>> - strncpy(sdiodev->nvram_name, brcmf_firmware_path, nv_len);
>> - fw_len -= strlen(sdiodev->fw_name);
>> - nv_len -= strlen(sdiodev->nvram_name);
>> + strlcpy(sdiodev->fw_name, brcmf_firmware_path,
>> + sizeof(sdiodev->fw_name));
>> + strlcpy(sdiodev->nvram_name, brcmf_firmware_path,
>> + sizeof(sdiodev->nvram_name));
>>
>> end = brcmf_firmware_path[strlen(brcmf_firmware_path) - 1];
>
> If you are doing a strlen() here, you could use the length for the copy
> and/or use it to avoid the strcat().
>
>> if (end != '/') {
>> - strncat(sdiodev->fw_name, "/", fw_len);
>> - strncat(sdiodev->nvram_name, "/", nv_len);
>> - fw_len--;
>> - nv_len--;
>> + strlcat(sdiodev->fw_name, "/",
>> + sizeof(sdiodev->fw_name));
>> + strlcat(sdiodev->nvram_name, "/",
>> + sizeof(sdiodev->nvram_name));
>> }
>> }
>> - strncat(sdiodev->fw_name, brcmf_fwname_data[i].bin, fw_len);
>> - strncat(sdiodev->nvram_name, brcmf_fwname_data[i].nv, nv_len);
>> + strlcat(sdiodev->fw_name, brcmf_fwname_data[i].bin,
>> + sizeof(sdiodev->fw_name));
>> + strlcat(sdiodev->nvram_name, brcmf_fwname_data[i].nv,
>> + sizeof(sdiodev->nvram_name));
>
> I assume something ensures that fw_name[0] == 0 here.
>
> David
Hi David
What do you mean you would use the strlen, can you give some example
code instead?
Arend van Spriel wanted me to change the title before.
So this should really continue the conversation in that new mail. And
then you can also see my other comments.
And I have the same type of objection to the:
brcmf_firmware_path[0] == '\0'
Se:
https://lkml.org/lkml/2014/10/12/42
Kind regards
Rickard Strandqvist
^ 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