* [PATCH net 2/2] l2tp: fix race between l2tp_session_delete() and l2tp_tunnel_closeall()
From: Guillaume Nault @ 2017-09-22 13:39 UTC (permalink / raw)
To: netdev; +Cc: James Chapman, Tom Parkin, Sabrina Dubroca
In-Reply-To: <cover.1506086081.git.g.nault@alphalink.fr>
There are several ways to remove L2TP sessions:
* deleting a session explicitly using the netlink interface (with
L2TP_CMD_SESSION_DELETE),
* deleting the session's parent tunnel (either by closing the
tunnel's file descriptor or using the netlink interface),
* closing the PPPOL2TP file descriptor of a PPP pseudo-wire.
In some cases, when these methods are used concurrently on the same
session, the session can be removed twice, leading to use-after-free
bugs.
This patch adds a 'dead' flag, used by l2tp_session_delete() and
l2tp_tunnel_closeall() to prevent them from stepping on each other's
toes.
The session deletion path used when closing a PPPOL2TP file descriptor
doesn't need to be adapted. It already has to ensure that a session
remains valid for the lifetime of its PPPOL2TP file descriptor.
So it takes an extra reference on the session in the ->session_close()
callback (pppol2tp_session_close()), which is eventually dropped
in the ->sk_destruct() callback of the PPPOL2TP socket
(pppol2tp_session_destruct()).
Still, __l2tp_session_unhash() and l2tp_session_queue_purge() can be
called twice and even concurrently for a given session, but thanks to
proper locking and re-initialisation of list fields, this is not an
issue.
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
net/l2tp/l2tp_core.c | 6 ++++++
net/l2tp/l2tp_core.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index ee485df73ccd..d8c2a89a76e1 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1314,6 +1314,9 @@ void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
hlist_del_init(&session->hlist);
+ if (test_and_set_bit(0, &session->dead))
+ goto again;
+
if (session->ref != NULL)
(*session->ref)(session);
@@ -1750,6 +1753,9 @@ EXPORT_SYMBOL_GPL(__l2tp_session_unhash);
*/
int l2tp_session_delete(struct l2tp_session *session)
{
+ if (test_and_set_bit(0, &session->dead))
+ return 0;
+
if (session->ref)
(*session->ref)(session);
__l2tp_session_unhash(session);
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a305e0c5925a..70a12df40a5f 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -76,6 +76,7 @@ struct l2tp_session_cfg {
struct l2tp_session {
int magic; /* should be
* L2TP_SESSION_MAGIC */
+ long dead;
struct l2tp_tunnel *tunnel; /* back pointer to tunnel
* context */
--
2.14.1
^ permalink raw reply related
* [PATCH net 1/2] l2tp: ensure sessions are freed after their PPPOL2TP socket
From: Guillaume Nault @ 2017-09-22 13:39 UTC (permalink / raw)
To: netdev; +Cc: James Chapman, Tom Parkin, Sabrina Dubroca
In-Reply-To: <cover.1506086081.git.g.nault@alphalink.fr>
If l2tp_tunnel_delete() or l2tp_tunnel_closeall() deletes a session
right after pppol2tp_release() orphaned its socket, then the 'sock'
variable of the pppol2tp_session_close() callback is NULL. Yet the
session is still used by pppol2tp_release().
Therefore we need to take an extra reference in any case, to prevent
l2tp_tunnel_delete() or l2tp_tunnel_closeall() from freeing the session.
Since the pppol2tp_session_close() callback is only set if the session
is associated to a PPPOL2TP socket and that both l2tp_tunnel_delete()
and l2tp_tunnel_closeall() hold the PPPOL2TP socket before calling
pppol2tp_session_close(), we're sure that pppol2tp_session_close() and
pppol2tp_session_destruct() are paired and called in the right order.
So the reference taken by the former will be released by the later.
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
net/l2tp/l2tp_ppp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 50e3ee9a9d61..bc6e8bfc5be4 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -437,11 +437,11 @@ static void pppol2tp_session_close(struct l2tp_session *session)
BUG_ON(session->magic != L2TP_SESSION_MAGIC);
- if (sock) {
+ if (sock)
inet_shutdown(sock, SEND_SHUTDOWN);
- /* Don't let the session go away before our socket does */
- l2tp_session_inc_refcount(session);
- }
+
+ /* Don't let the session go away before our socket does */
+ l2tp_session_inc_refcount(session);
}
/* Really kill the session socket. (Called from sock_put() if
--
2.14.1
^ permalink raw reply related
* [PATCH net 0/2] l2tp: fix some races in session deletion
From: Guillaume Nault @ 2017-09-22 13:39 UTC (permalink / raw)
To: netdev; +Cc: James Chapman, Tom Parkin, Sabrina Dubroca
L2TP provides several interfaces for deleting sessions. Using two of
them concurrently can lead to use-after-free bugs.
Patch #2 uses a flag to prevent double removal of L2TP sessions.
Patch #1 fixes a bug found in the way. Fixing this bug is also
necessary for patch #2 to handle all cases.
This issue is similar to the tunnel deletion bug being worked on by
Sabrina: https://patchwork.ozlabs.org/patch/814173/
Guillaume Nault (2):
l2tp: ensure sessions are freed after their PPPOL2TP socket
l2tp: fix race between l2tp_session_delete() and
l2tp_tunnel_closeall()
net/l2tp/l2tp_core.c | 6 ++++++
net/l2tp/l2tp_core.h | 1 +
net/l2tp/l2tp_ppp.c | 8 ++++----
3 files changed, 11 insertions(+), 4 deletions(-)
--
2.14.1
^ permalink raw reply
* Re: [patch net-next 07/12] mlxsw: spectrum: Add the multicast routing offloading logic
From: Andrew Lunn @ 2017-09-22 13:21 UTC (permalink / raw)
To: Yotam Gigi; +Cc: Jiri Pirko, netdev, davem, idosch, mlxsw
In-Reply-To: <a6852db1-76a2-b4f3-c558-cd4cc0a1309b@mellanox.com>
On Fri, Sep 22, 2017 at 11:36:59AM +0300, Yotam Gigi wrote:
> On 09/21/2017 06:26 PM, Andrew Lunn wrote:
> >> +static void mlxsw_sp_mr_route_stats_update(struct mlxsw_sp *mlxsw_sp,
> >> + struct mlxsw_sp_mr_route *mr_route)
> >> +{
> >> + struct mlxsw_sp_mr *mr = mlxsw_sp->mr;
> >> + u64 packets, bytes;
> >> +
> >> + if (mr_route->route_action == MLXSW_SP_MR_ROUTE_ACTION_TRAP)
> >> + return;
> >> +
> >> + mr->mr_ops->route_stats(mlxsw_sp, mr_route->route_priv, &packets,
> >> + &bytes);
> >> +
> >> + switch (mr_route->mr_table->proto) {
> >> + case MLXSW_SP_L3_PROTO_IPV4:
> >> + mr_route->mfc4->mfc_un.res.pkt = packets;
> >> + mr_route->mfc4->mfc_un.res.bytes = bytes;
> > What about wrong_if and lastuse?
Hi Yotam
> wronf_if is updated by ipmr as it is trapped to the CPU.
Great.
> We did not address lastuse currently, though it can be easily
> addressed here.
Please do. I've written multicast routing daemons, where i use it to
flush out MFCs which are no longer in use. Having it always 0 is going
to break daemons.
> > Is an mfc with iif on the host, not the switch, not offloaded?
>
>
> I am not sure I followed. What do you mean MFC with iif on the host? you mean
> MFC with iif that is an external NIC which is not part of the spectrum ASIC?
Yes. We probably have different perspectives on the world. To
Mellanox, everything is a switch in a box. In the DSA world, we tend
to think of having a general purpose machine which also has a switch
connected. Think of a wireless access point, set top box, passenger
entertainment system. We have applications on the general purpose
computer, we have wifi interfaces, cable modems, etc. Think about all
the different packages in LEDE. We might have a multicast video
stream, coming from the cable modem being sent over ports of the
switch to clients.
So when i look at these patches, i try to make sure the general use
cases works, not just the plain boring Ethernet switch box use cases
:-)
> in this case, the route will not be offloaded and all traffic will
> pass in slowpath.
O.K. I was just thinking if those counters need to be +=, not =. But
either the iif is on the host, or it is in the switch. It cannot be
both. So = is O.K.
Thanks
Andrew
^ permalink raw reply
* Re: tg3 pxe weirdness
From: Berend De Schouwer @ 2017-09-22 13:20 UTC (permalink / raw)
To: Siva Reddy Kallam; +Cc: Linux Netdev List
In-Reply-To: <CAMet4B4Jdgjs42KAWQ4HJr1t_Grn1_+Hv8sipeLb_tvZ_SFrMw@mail.gmail.com>
On Fri, 2017-09-22 at 11:51 +0530, Siva Reddy Kallam wrote:
> On Thu, Sep 21, 2017 at 7:53 PM, Berend De Schouwer
> <berend.de.schouwer@gmail.com> wrote:
> > Hi,
> >
> > I've got a machine with a Broadcom bcm5762c, using the tg3 driver,
> > that
> > fails to receive network packets under some very specific
> > conditions.
> >
> >
> > Berend
>
> Can you please share below details?
> 1) Model and Manufacturer of the system
> 2) Linux distro/kernel used?
3.10.107 behaves like CentOS' kernel.
^ permalink raw reply
* RE: [net-next 1/2] dummy: add device MTU validation check
From: 张胜举 @ 2017-09-22 12:59 UTC (permalink / raw)
To: 'Sabrina Dubroca', 'Eric Dumazet'
Cc: 'Jarod Wilson', davem, willemb, stephen, netdev
In-Reply-To: <20170922122315.GA3446@bistromath.localdomain>
> -----Original Message-----
> From: Sabrina Dubroca [mailto:sd@queasysnail.net]
> Sent: 2017年9月22日 20:23
> To: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Jarod Wilson <jarod@redhat.com>; Zhang Shengju
> <zhangshengju@cmss.chinamobile.com>; davem@davemloft.net;
> willemb@google.com; stephen@networkplumber.org;
> netdev@vger.kernel.org
> Subject: Re: [net-next 1/2] dummy: add device MTU validation check
>
> 2017-09-22, 04:05:09 -0700, Eric Dumazet wrote:
> > On Fri, 2017-09-22 at 10:56 +0200, Sabrina Dubroca wrote:
> > > 2017-09-21, 08:02:18 -0700, Eric Dumazet wrote:
> > > > On Thu, 2017-09-21 at 21:32 +0800, Zhang Shengju wrote:
> > > > > Currently, any mtu value can be assigned when adding a new dummy
> device:
> > > > > [~]# ip link add name dummy1 mtu 100000 type dummy [~]# ip link
> > > > > show dummy1
> > > > > 15: dummy1: <BROADCAST,NOARP> mtu 100000 qdisc noop state
> DOWN mode DEFAULT group default qlen 1000
> > > > > link/ether 0a:61:6b:16:14:ce brd ff:ff:ff:ff:ff:ff
> > > > >
> > > > > This patch adds device MTU validation check.
> > > >
> > > > What is wrong with big MTU on dummy ?
> > >
> > > It looks like the "centralize MTU checking" series broke that, but
> > > only for changing the MTU on an existing dummy device. Commit
> > > a52ad514fdf3 defined min_mtu/max_mtu in ether_setup, which dummy
> > > uses, but there was no MTU check in dummy prior to that commit.
> > >
> >
> > It looks like we accept big mtu on loopback, right ?
>
> Yes. I only meant that before commit a52ad514fdf3, there was no range check
> on dummy's MTU. Commit 25e3e84b183a ("dummy: expend mtu range for
> dummy device") and 8b1efc0f83f1 ("net: remove MTU limits on a few
> ether_setup callers") fixed that only partially. It's the same with ifb, btw, it
> didn't have any check before a52ad514fdf3, so we should set min_mtu =
> max_mtu = 0.
>
> --
> Sabrina
I agree, dummy and ifb device should not have any limit on mtu, just like loopback device.
I will send v2 patch, and set min/max_mtu to zero for dummy and ifb device, thanks.
ZSJ
^ permalink raw reply
* Re: tg3 pxe weirdness
From: Berend De Schouwer @ 2017-09-22 12:56 UTC (permalink / raw)
To: Siva Reddy Kallam; +Cc: Linux Netdev List
In-Reply-To: <CAMet4B4Jdgjs42KAWQ4HJr1t_Grn1_+Hv8sipeLb_tvZ_SFrMw@mail.gmail.com>
On Fri, 2017-09-22 at 11:51 +0530, Siva Reddy Kallam wrote:
> On Thu, Sep 21, 2017 at 7:53 PM, Berend De Schouwer
> <berend.de.schouwer@gmail.com> wrote:
> > Hi,
> >
> > I've got a machine with a Broadcom bcm5762c, using the tg3 driver,
> > that
> > fails to receive network packets under some very specific
> > conditions.
> >
> >
> > Berend
>
> Can you please share below details?
> 1) Model and Manufacturer of the system
> 2) Linux distro/kernel used?
4.13.3 mainline locks up, but network dumps confirm it gets further.
The second DHCP query completes, and the first NFS mount completes.
I have to rely on network dumps since on PXE and SATA boot it breaks
VGA output, so I can't see what it's doing, or why it stops.
^ permalink raw reply
* Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when interacting with network stack
From: Jiri Pirko @ 2017-09-22 12:55 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, huangdaode, xuwei5, liguozhu, Yisen.Zhuang,
gabriele.paoloni, john.garry, linuxarm, salil.mehta, lipeng321,
netdev, linux-kernel
In-Reply-To: <1505992913-107256-11-git-send-email-linyunsheng@huawei.com>
Thu, Sep 21, 2017 at 01:21:53PM CEST, linyunsheng@huawei.com wrote:
>When using tc qdisc to configure DCB parameter, dcb_ops->setup_tc
>is used to tell hclge_dcb module to do the setup.
>When using lldptool to configure DCB parameter, hclge_dcb module
>call the client_ops->setup_tc to tell network stack which queue
>and priority is using for specific tc.
>
>Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
[...]
>-static int hns3_setup_tc(struct net_device *netdev, u8 tc)
>+static int hns3_setup_tc(struct net_device *netdev, u8 tc, u8 *prio_tc)
> {
> struct hns3_nic_priv *priv = netdev_priv(netdev);
> struct hnae3_handle *h = priv->ae_handle;
> struct hnae3_knic_private_info *kinfo = &h->kinfo;
>+ bool if_running = netif_running(netdev);
> unsigned int i;
> int ret;
>
> if (tc > HNAE3_MAX_TC)
> return -EINVAL;
>
>- if (kinfo->num_tc == tc)
>- return 0;
>-
> if (!netdev)
> return -EINVAL;
>
>- if (!tc) {
>+ if (if_running) {
>+ (void)hns3_nic_net_stop(netdev);
>+ msleep(100);
>+ }
>+
>+ ret = (kinfo->dcb_ops && kinfo->dcb_ops->setup_tc) ?
>+ kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : -EOPNOTSUPP;
This is most odd. Why do you call dcb_ops from ndo_setup_tc callback?
Why are you mixing this together? prio->tc mapping can be done
directly in dcbnl
^ permalink raw reply
* [PATCH] MAINTAINERS: update git tree locations for ieee802154 subsystem
From: Stefan Schmidt @ 2017-09-22 12:28 UTC (permalink / raw)
To: davem, linux-wpan; +Cc: Alexander Aring, netdev, Stefan Schmidt
Patches for ieee802154 will go through my new trees towards netdev from
now on. The 6LoWPAN subsystem will stay as is (shared between ieee802154
and bluetooth) and go through the bluetooth tree as usual.
Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
MAINTAINERS | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2281af4b41b6..fc7313daf4b0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6643,8 +6643,8 @@ M: Alexander Aring <alex.aring@gmail.com>
M: Stefan Schmidt <stefan@osg.samsung.com>
L: linux-wpan@vger.kernel.org
W: http://wpan.cakelab.org/
-T: git git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git
-T: git git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan.git
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan-next.git
S: Maintained
F: net/ieee802154/
F: net/mac802154/
--
2.13.5
^ permalink raw reply related
* Re: [net-next 1/2] dummy: add device MTU validation check
From: Sabrina Dubroca @ 2017-09-22 12:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jarod Wilson, Zhang Shengju, davem, willemb, stephen, netdev
In-Reply-To: <1506078309.29839.161.camel@edumazet-glaptop3.roam.corp.google.com>
2017-09-22, 04:05:09 -0700, Eric Dumazet wrote:
> On Fri, 2017-09-22 at 10:56 +0200, Sabrina Dubroca wrote:
> > 2017-09-21, 08:02:18 -0700, Eric Dumazet wrote:
> > > On Thu, 2017-09-21 at 21:32 +0800, Zhang Shengju wrote:
> > > > Currently, any mtu value can be assigned when adding a new dummy device:
> > > > [~]# ip link add name dummy1 mtu 100000 type dummy
> > > > [~]# ip link show dummy1
> > > > 15: dummy1: <BROADCAST,NOARP> mtu 100000 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> > > > link/ether 0a:61:6b:16:14:ce brd ff:ff:ff:ff:ff:ff
> > > >
> > > > This patch adds device MTU validation check.
> > >
> > > What is wrong with big MTU on dummy ?
> >
> > It looks like the "centralize MTU checking" series broke that, but
> > only for changing the MTU on an existing dummy device. Commit
> > a52ad514fdf3 defined min_mtu/max_mtu in ether_setup, which dummy uses,
> > but there was no MTU check in dummy prior to that commit.
> >
>
> It looks like we accept big mtu on loopback, right ?
Yes. I only meant that before commit a52ad514fdf3, there was no range
check on dummy's MTU. Commit 25e3e84b183a ("dummy: expend mtu range
for dummy device") and 8b1efc0f83f1 ("net: remove MTU limits on a few
ether_setup callers") fixed that only partially. It's the same with
ifb, btw, it didn't have any check before a52ad514fdf3, so we should
set min_mtu = max_mtu = 0.
--
Sabrina
^ permalink raw reply
* Re: [PATCH] e1000: avoid null pointer dereference on invalid stat type
From: Colin Ian King @ 2017-09-22 11:55 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jeff Kirsher, intel-wired-lan, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20170922115038.ckvl6zsmadrqiige@mwanda>
On 22/09/17 12:50, Dan Carpenter wrote:
> On Thu, Sep 21, 2017 at 11:01:58PM +0100, Colin King wrote:
>> @@ -1837,12 +1838,13 @@ static void e1000_get_ethtool_stats(struct net_device *netdev,
>> p = (char *)adapter + stat->stat_offset;
>> break;
>> default:
>> + p = NULL;
>> WARN_ONCE(1, "Invalid E1000 stat type: %u index %d\n",
>> stat->type, i);
>> break;
>> }
>>
>> - if (stat->sizeof_stat == sizeof(u64))
>> + if (p && stat->sizeof_stat == sizeof(u64))
>> data[i] = *(u64 *)p;
>> else
>> data[i] = *(u32 *)p;
> ^^^^^^^^
>
> The else side will still crash.
>
> regards,
> dan carpenter
>
Thanks, stupid me. I'll fix that up.
^ permalink raw reply
* Re: [PATCH] e1000: avoid null pointer dereference on invalid stat type
From: Dan Carpenter @ 2017-09-22 11:50 UTC (permalink / raw)
To: Colin King
Cc: Jeff Kirsher, intel-wired-lan, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20170921220158.19341-1-colin.king@canonical.com>
On Thu, Sep 21, 2017 at 11:01:58PM +0100, Colin King wrote:
> @@ -1837,12 +1838,13 @@ static void e1000_get_ethtool_stats(struct net_device *netdev,
> p = (char *)adapter + stat->stat_offset;
> break;
> default:
> + p = NULL;
> WARN_ONCE(1, "Invalid E1000 stat type: %u index %d\n",
> stat->type, i);
> break;
> }
>
> - if (stat->sizeof_stat == sizeof(u64))
> + if (p && stat->sizeof_stat == sizeof(u64))
> data[i] = *(u64 *)p;
> else
> data[i] = *(u32 *)p;
^^^^^^^^
The else side will still crash.
regards,
dan carpenter
^ permalink raw reply
* [PATCH iproute2 v2] man: fix documentation for range of route table ID
From: Thomas Haller @ 2017-09-22 11:28 UTC (permalink / raw)
To: Stephen Hemminger, netdev; +Cc: Phil Sutter, Thomas Haller
In-Reply-To: <20170921182647.GB30364@orbyte.nwl.cc>
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
Changes in v2:
- "0" is not a valid table ID.
man/man8/ip-route.8.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index 803de3b9..705ceb20 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -322,7 +322,7 @@ normal routing tables.
.P
.B Route tables:
Linux-2.x can pack routes into several routing tables identified
-by a number in the range from 1 to 2^31 or by name from the file
+by a number in the range from 1 to 2^32-1 or by name from the file
.B @SYSCONFDIR@/rt_tables
By default all normal routes are inserted into the
.B main
--
2.13.5
^ permalink raw reply related
* [PATCH] net: stmmac: Meet alignment requirements for DMA
From: Matt Redfearn @ 2017-09-22 11:13 UTC (permalink / raw)
To: David S . Miller
Cc: Matt Redfearn, netdev, Alexandre Torgue, Giuseppe Cavallaro,
linux-kernel
According to Documentation/DMA-API.txt:
Warnings: Memory coherency operates at a granularity called the cache
line width. In order for memory mapped by this API to operate
correctly, the mapped region must begin exactly on a cache line
boundary and end exactly on one (to prevent two separately mapped
regions from sharing a single cache line). Since the cache line size
may not be known at compile time, the API will not enforce this
requirement. Therefore, it is recommended that driver writers who
don't take special care to determine the cache line size at run time
only map virtual regions that begin and end on page boundaries (which
are guaranteed also to be cache line boundaries).
On some systems where DMA is non-coherent and requires software
writeback / invalidate of the caches, we must ensure that
dma_(un)map_single is called with a cacheline aligned buffer and a
length of a whole number of cachelines.
To address the alignment requirements of DMA buffers, keep a separate
entry in stmmac_rx_queue for the aligned skbuff head. Use this for
dma_map_single, such that the address meets the cacheline alignment
requirents. Use skb_headroom() to convert between rx_skbuff_head, the
aligned head of the buffer, and the packet data, rx_skbuff_dma.
Tested on a Creator Ci40 with Pistachio SoC.
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 +
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 50 ++++++++++++++++-------
2 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index a916e13624eb..dd26a724dee7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -67,6 +67,7 @@ struct stmmac_rx_queue {
struct dma_desc *dma_rx ____cacheline_aligned_in_smp;
struct sk_buff **rx_skbuff;
dma_addr_t *rx_skbuff_dma;
+ dma_addr_t *rx_skbuff_head;
unsigned int cur_rx;
unsigned int dirty_rx;
u32 rx_zeroc_thresh;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1763e48c84e2..da68eeff2a1c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1132,14 +1132,16 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, struct dma_desc *p,
return -ENOMEM;
}
rx_q->rx_skbuff[i] = skb;
- rx_q->rx_skbuff_dma[i] = dma_map_single(priv->device, skb->data,
- priv->dma_buf_sz,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(priv->device, rx_q->rx_skbuff_dma[i])) {
+ rx_q->rx_skbuff_head[i] = dma_map_single(priv->device, skb->head,
+ skb_headroom(skb) +
+ priv->dma_buf_sz,
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(priv->device, rx_q->rx_skbuff_head[i])) {
netdev_err(priv->dev, "%s: DMA mapping error\n", __func__);
dev_kfree_skb_any(skb);
return -EINVAL;
}
+ rx_q->rx_skbuff_dma[i] = rx_q->rx_skbuff_head[i] + skb_headroom(skb);
if (priv->synopsys_id >= DWMAC_CORE_4_00)
p->des0 = cpu_to_le32(rx_q->rx_skbuff_dma[i]);
@@ -1164,7 +1166,8 @@ static void stmmac_free_rx_buffer(struct stmmac_priv *priv, u32 queue, int i)
struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
if (rx_q->rx_skbuff[i]) {
- dma_unmap_single(priv->device, rx_q->rx_skbuff_dma[i],
+ dma_unmap_single(priv->device, rx_q->rx_skbuff_head[i],
+ skb_headroom(rx_q->rx_skbuff[i]) +
priv->dma_buf_sz, DMA_FROM_DEVICE);
dev_kfree_skb_any(rx_q->rx_skbuff[i]);
}
@@ -1438,6 +1441,7 @@ static void free_dma_rx_desc_resources(struct stmmac_priv *priv)
rx_q->dma_erx, rx_q->dma_rx_phy);
kfree(rx_q->rx_skbuff_dma);
+ kfree(rx_q->rx_skbuff_head);
kfree(rx_q->rx_skbuff);
}
}
@@ -1500,6 +1504,12 @@ static int alloc_dma_rx_desc_resources(struct stmmac_priv *priv)
if (!rx_q->rx_skbuff_dma)
goto err_dma;
+ rx_q->rx_skbuff_head = kmalloc_array(DMA_RX_SIZE,
+ sizeof(dma_addr_t),
+ GFP_KERNEL);
+ if (!rx_q->rx_skbuff_head)
+ goto err_dma;
+
rx_q->rx_skbuff = kmalloc_array(DMA_RX_SIZE,
sizeof(struct sk_buff *),
GFP_KERNEL);
@@ -3225,15 +3235,18 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
}
rx_q->rx_skbuff[entry] = skb;
- rx_q->rx_skbuff_dma[entry] =
- dma_map_single(priv->device, skb->data, bfsize,
+ rx_q->rx_skbuff_head[entry] =
+ dma_map_single(priv->device, skb->head,
+ skb_headroom(skb) + bfsize,
DMA_FROM_DEVICE);
if (dma_mapping_error(priv->device,
- rx_q->rx_skbuff_dma[entry])) {
+ rx_q->rx_skbuff_head[entry])) {
netdev_err(priv->dev, "Rx DMA map failed\n");
dev_kfree_skb(skb);
break;
}
+ rx_q->rx_skbuff_dma[entry] = rx_q->rx_skbuff_head[entry]
+ + skb_headroom(skb);
if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00)) {
p->des0 = cpu_to_le32(rx_q->rx_skbuff_dma[entry]);
@@ -3333,10 +3346,12 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
* them in stmmac_rx_refill() function so that
* device can reuse it.
*/
+ int head = skb_headroom(rx_q->rx_skbuff[entry]);
+
rx_q->rx_skbuff[entry] = NULL;
dma_unmap_single(priv->device,
- rx_q->rx_skbuff_dma[entry],
- priv->dma_buf_sz,
+ rx_q->rx_skbuff_head[entry],
+ head + priv->dma_buf_sz,
DMA_FROM_DEVICE);
}
} else {
@@ -3384,6 +3399,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
if (unlikely(!priv->plat->has_gmac4 &&
((frame_len < priv->rx_copybreak) ||
stmmac_rx_threshold_count(rx_q)))) {
+ int total_len;
+
skb = netdev_alloc_skb_ip_align(priv->dev,
frame_len);
if (unlikely(!skb)) {
@@ -3394,9 +3411,11 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
break;
}
+ total_len = skb_headroom(skb) + frame_len;
+
dma_sync_single_for_cpu(priv->device,
- rx_q->rx_skbuff_dma
- [entry], frame_len,
+ rx_q->rx_skbuff_head
+ [entry], total_len,
DMA_FROM_DEVICE);
skb_copy_to_linear_data(skb,
rx_q->
@@ -3405,8 +3424,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
skb_put(skb, frame_len);
dma_sync_single_for_device(priv->device,
- rx_q->rx_skbuff_dma
- [entry], frame_len,
+ rx_q->rx_skbuff_head
+ [entry], total_len,
DMA_FROM_DEVICE);
} else {
skb = rx_q->rx_skbuff[entry];
@@ -3423,7 +3442,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
skb_put(skb, frame_len);
dma_unmap_single(priv->device,
- rx_q->rx_skbuff_dma[entry],
+ rx_q->rx_skbuff_head[entry],
+ skb_headroom(skb) +
priv->dma_buf_sz,
DMA_FROM_DEVICE);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] net: mvpp2: phylink support
From: Russell King - ARM Linux @ 2017-09-22 11:07 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, andrew, gregory.clement, thomas.petazzoni, miquel.raynal,
nadavh, linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170921134522.10993-1-antoine.tenart@free-electrons.com>
On Thu, Sep 21, 2017 at 03:45:22PM +0200, Antoine Tenart wrote:
> Convert the PPv2 driver to use phylink, which models the MAC to PHY
> link. The phylink support is made such a way the GoP link IRQ can still
> be used: the two modes are incompatible and the GoP link IRQ will be
> used if no PHY is described in the device tree. This is the same
> behaviour as before.
This makes no sense. The point of phylink is to be able to support SFP
cages, and SFP cages do not have a PHY described in DT. So, when you
want to use phylink because of SFP, you can't, because if you omit
the PHY the driver avoids using phylink.
> +static void mvpp2_phylink_validate(struct net_device *dev,
> + unsigned long *supported,
> + struct phylink_link_state *state)
> +{
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> + phylink_set_port_modes(mask);
> +
> + phylink_set(mask, Autoneg);
> + phylink_set(mask, Pause);
> + phylink_set(mask, Asym_Pause);
> +
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> + phylink_set(mask, 1000baseT_Half);
> + phylink_set(mask, 1000baseT_Full);
> + phylink_set(mask, 1000baseX_Full);
> + phylink_set(mask, 10000baseKR_Full);
> +
> + bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + bitmap_and(state->advertising, state->advertising, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> +}
I don't think you've understood this despite the comments in the header
file. What you describe above basically means you don't support any
kind of copper connection at 10G speeds, or any fiber modes at 10G
speeds either.
You need to set 10000baseT_Full for copper, 10000baseSR_Full,
10000baseLR_Full, 10000baseLRM_Full etc for fiber. Had you looked at
my modifications for Marvell's mvpp2x driver you'd have spotted this...
> +
> +static int mvpp2_phylink_mac_link_state(struct net_device *dev,
> + struct phylink_link_state *state)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return 0;
You're blocking this for 1000base-X and 10G connections, which is not
correct. The expectation is that this function returns the current
MAC state irrespective of the interface mode.
> +
> + val = readl(port->base + MVPP2_GMAC_STATUS0);
> +
> + state->an_complete = !!(val & MVPP2_GMAC_STATUS0_AN_COMPLETE);
> + state->link = !!(val & MVPP2_GMAC_STATUS0_LINK_UP);
> + state->duplex = !!(val & MVPP2_GMAC_STATUS0_FULL_DUPLEX);
> +
> + if (val & MVPP2_GMAC_STATUS0_GMII_SPEED)
> + state->speed = SPEED_1000;
> + else
> + state->speed = (val & MVPP2_GMAC_STATUS0_MII_SPEED) ?
> + SPEED_100 : SPEED_10;
> +
> + state->pause = 0;
> + if (val & MVPP2_GMAC_STATUS0_RX_PAUSE)
> + state->pause |= MLO_PAUSE_RX;
> + if (val & MVPP2_GMAC_STATUS0_TX_PAUSE)
> + state->pause |= MLO_PAUSE_TX;
> +
> + return 1;
> +}
> +
> +static void mvpp2_mac_an_restart(struct net_device *dev)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return;
This prevents AN restart in 1000base-X mode, which is exactly the
mode that you need to do this. SGMII doesn't care, and RGMII doesn't
have inband AN.
> +
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val |= MVPP2_GMAC_IN_BAND_RESTART_AN;
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +}
> +
> +static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
> + const struct phylink_link_state *state)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + /* disable current port for reconfiguration */
> + mvpp2_interrupts_disable(port);
> + netif_carrier_off(port->dev);
> + mvpp2_port_disable(port);
> + phy_power_off(port->comphy);
> +
> + /* comphy reconfiguration */
> + port->phy_interface = state->interface;
> + mvpp22_comphy_init(port);
> +
> + /* gop/mac reconfiguration */
> + mvpp22_gop_init(port);
> + mvpp2_port_mii_set(port);
> +
> + if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
> + port->phy_interface != PHY_INTERFACE_MODE_SGMII)
> + return;
Again, 1000base-X is excluded, which will break it. You do need
to avoid touching the GMAC for 10G connections however.
> +
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val &= ~(MVPP2_GMAC_CONFIG_MII_SPEED |
> + MVPP2_GMAC_CONFIG_GMII_SPEED |
> + MVPP2_GMAC_CONFIG_FULL_DUPLEX |
> + MVPP2_GMAC_AN_SPEED_EN |
> + MVPP2_GMAC_AN_DUPLEX_EN);
> +
> + if (state->duplex)
> + val |= MVPP2_GMAC_CONFIG_FULL_DUPLEX;
> +
> + if (state->speed == SPEED_1000)
> + val |= MVPP2_GMAC_CONFIG_GMII_SPEED;
> + else if (state->speed == SPEED_100)
> + val |= MVPP2_GMAC_CONFIG_MII_SPEED;
> +
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
You're assuming that this function only sets the current parameters for
the MAC. That's incorrect - it also needs to deal with autonegotiation
for inband AN, such as SGMII and 1000base-X.
> +
> + if (port->priv->hw_version == MVPP21 && port->flags & MVPP2_F_LOOPBACK)
> + mvpp2_port_loopback_set(port, state);
> +}
> +
> +static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode)
> +{
> + struct mvpp2_port *port = netdev_priv(dev);
> + u32 val;
> +
> + netif_tx_stop_all_queues(dev);
> + netif_carrier_off(dev);
> + mvpp2_ingress_disable(port);
> + mvpp2_egress_disable(port);
> +
> + mvpp2_port_disable(port);
> + mvpp2_interrupts_disable(port);
> +
> + if (!phylink_autoneg_inband(mode)) {
> + val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + val &= ~MVPP2_GMAC_FORCE_LINK_PASS;
> + val |= MVPP2_GMAC_FORCE_LINK_DOWN;
> + writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> + }
Please explain why you think its necessary to force the link down when
the link is already down - if there's no media connected, we only
need to stop the ingress and egress.
It's certainly wrong to disable interrupts - how do we end up with
link status changes reported from the MAC to phylink if interrupts
have been disabled?
phylink in the presence of a PHY checks that both the PHY _and_ MAC
are reporting link before it calls mac_link_up(), so if you've
disabled interrupts...
You guys know that I have working example code for both mvneta and the
Marvell PP2x driver. It probably would help if you looked at those
examples.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [net-next 1/2] dummy: add device MTU validation check
From: Eric Dumazet @ 2017-09-22 11:05 UTC (permalink / raw)
To: Sabrina Dubroca
Cc: Jarod Wilson, Zhang Shengju, davem, willemb, stephen, netdev
In-Reply-To: <20170922085610.GA4544@bistromath.localdomain>
On Fri, 2017-09-22 at 10:56 +0200, Sabrina Dubroca wrote:
> 2017-09-21, 08:02:18 -0700, Eric Dumazet wrote:
> > On Thu, 2017-09-21 at 21:32 +0800, Zhang Shengju wrote:
> > > Currently, any mtu value can be assigned when adding a new dummy device:
> > > [~]# ip link add name dummy1 mtu 100000 type dummy
> > > [~]# ip link show dummy1
> > > 15: dummy1: <BROADCAST,NOARP> mtu 100000 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> > > link/ether 0a:61:6b:16:14:ce brd ff:ff:ff:ff:ff:ff
> > >
> > > This patch adds device MTU validation check.
> >
> > What is wrong with big MTU on dummy ?
>
> It looks like the "centralize MTU checking" series broke that, but
> only for changing the MTU on an existing dummy device. Commit
> a52ad514fdf3 defined min_mtu/max_mtu in ether_setup, which dummy uses,
> but there was no MTU check in dummy prior to that commit.
>
It looks like we accept big mtu on loopback, right ?
lpaa23:~# ifconfig lo mtu 100000
lpaa23:~# ifconfig lo
lo Link encap:Local Loopback
inet addr:127.0.0.1 Mask:255.0.0.0
inet6 addr: ::1/128 Scope:Host
UP LOOPBACK RUNNING MTU:100000 Metric:1
RX packets:3823 errors:0 dropped:0 overruns:0 frame:0
TX packets:3823 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:759159 (759.1 KB) TX bytes:759159 (759.1 KB)
Also we accept very small MTU as well (although this automatically
removes IP addresses, as one would expect)
lpaa23:~# ifconfig lo mtu 50
lpaa23:~# ifconfig lo
lo Link encap:Local Loopback
UP LOOPBACK RUNNING MTU:50 Metric:1
RX packets:4052 errors:0 dropped:0 overruns:0 frame:0
TX packets:4052 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:806274 (806.2 KB) TX bytes:806274 (806.2 KB)
So, why dummy devices would not accept bit MTU ?
Do we have some fundamental assumption in the stack ?
If yes, we need to fix loopback urgently, it is more important than
dummy.
Thanks.
^ permalink raw reply
* [PATCH 5/5] net: nfc: llcp_core: use setup_timer() helper.
From: Allen Pais @ 2017-09-22 10:58 UTC (permalink / raw)
To: netdev; +Cc: davem, sameo, Allen Pais
In-Reply-To: <1506077902-1796-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
net/nfc/llcp_core.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 02eef5c..7988185 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1573,9 +1573,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
INIT_LIST_HEAD(&local->list);
kref_init(&local->ref);
mutex_init(&local->sdp_lock);
- init_timer(&local->link_timer);
- local->link_timer.data = (unsigned long) local;
- local->link_timer.function = nfc_llcp_symm_timer;
+ setup_timer(&local->link_timer, nfc_llcp_symm_timer,
+ (unsigned long)local);
skb_queue_head_init(&local->tx_queue);
INIT_WORK(&local->tx_work, nfc_llcp_tx_work);
@@ -1601,9 +1600,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
mutex_init(&local->sdreq_lock);
INIT_HLIST_HEAD(&local->pending_sdreqs);
- init_timer(&local->sdreq_timer);
- local->sdreq_timer.data = (unsigned long) local;
- local->sdreq_timer.function = nfc_llcp_sdreq_timer;
+ setup_timer(&local->sdreq_timer, nfc_llcp_sdreq_timer,
+ (unsigned long)local);
INIT_WORK(&local->sdreq_timeout_work, nfc_llcp_sdreq_timeout_work);
list_add(&local->list, &llcp_devices);
--
2.7.4
^ permalink raw reply related
* [PATCH 4/5] net: nfc: core: use setup_timer() helper.
From: Allen Pais @ 2017-09-22 10:58 UTC (permalink / raw)
To: netdev; +Cc: davem, sameo, Allen Pais
In-Reply-To: <1506077902-1796-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
net/nfc/core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/nfc/core.c b/net/nfc/core.c
index 5cf33df..e5e23c2 100644
--- a/net/nfc/core.c
+++ b/net/nfc/core.c
@@ -1094,9 +1094,8 @@ struct nfc_dev *nfc_allocate_device(struct nfc_ops *ops,
dev->targets_generation = 1;
if (ops->check_presence) {
- init_timer(&dev->check_pres_timer);
- dev->check_pres_timer.data = (unsigned long)dev;
- dev->check_pres_timer.function = nfc_check_pres_timeout;
+ setup_timer(&dev->check_pres_timer, nfc_check_pres_timeout,
+ (unsigned long)dev);
INIT_WORK(&dev->check_pres_work, nfc_check_pres_work);
}
--
2.7.4
^ permalink raw reply related
* [PATCH 3/5] net: af_packet: use setup_timer() helper.
From: Allen Pais @ 2017-09-22 10:58 UTC (permalink / raw)
To: netdev; +Cc: davem, sameo, Allen Pais
In-Reply-To: <1506077902-1796-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
net/packet/af_packet.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index c261729..1d3e3ce 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -544,9 +544,7 @@ static void prb_init_blk_timer(struct packet_sock *po,
struct tpacket_kbdq_core *pkc,
void (*func) (unsigned long))
{
- init_timer(&pkc->retire_blk_timer);
- pkc->retire_blk_timer.data = (long)po;
- pkc->retire_blk_timer.function = func;
+ setup_timer(&pkc->retire_blk_timer, func, (long)po);
pkc->retire_blk_timer.expires = jiffies;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 2/5] net: nfc: hci: llc_shdlc: use setup_timer() helper.
From: Allen Pais @ 2017-09-22 10:58 UTC (permalink / raw)
To: netdev; +Cc: davem, sameo, Allen Pais
In-Reply-To: <1506077902-1796-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
net/nfc/hci/llc_shdlc.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/net/nfc/hci/llc_shdlc.c b/net/nfc/hci/llc_shdlc.c
index 17e59a0..58df37e 100644
--- a/net/nfc/hci/llc_shdlc.c
+++ b/net/nfc/hci/llc_shdlc.c
@@ -763,17 +763,14 @@ static void *llc_shdlc_init(struct nfc_hci_dev *hdev, xmit_to_drv_t xmit_to_drv,
mutex_init(&shdlc->state_mutex);
shdlc->state = SHDLC_DISCONNECTED;
- init_timer(&shdlc->connect_timer);
- shdlc->connect_timer.data = (unsigned long)shdlc;
- shdlc->connect_timer.function = llc_shdlc_connect_timeout;
+ setup_timer(&shdlc->connect_timer, llc_shdlc_connect_timeout,
+ (unsigned long)shdlc);
- init_timer(&shdlc->t1_timer);
- shdlc->t1_timer.data = (unsigned long)shdlc;
- shdlc->t1_timer.function = llc_shdlc_t1_timeout;
+ setup_timer(&shdlc->t1_timer, llc_shdlc_t1_timeout,
+ (unsigned long)shdlc);
- init_timer(&shdlc->t2_timer);
- shdlc->t2_timer.data = (unsigned long)shdlc;
- shdlc->t2_timer.function = llc_shdlc_t2_timeout;
+ setup_timer(&shdlc->t2_timer, llc_shdlc_t2_timeout,
+ (unsigned long)shdlc);
shdlc->w = SHDLC_MAX_WINDOW;
shdlc->srej_support = SHDLC_SREJ_SUPPORT;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/5] net: nfc: hci: use setup_timer() helper.
From: Allen Pais @ 2017-09-22 10:58 UTC (permalink / raw)
To: netdev; +Cc: davem, sameo, Allen Pais
In-Reply-To: <1506077902-1796-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
net/nfc/hci/core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index b740fef..a8a6e78 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -1004,9 +1004,8 @@ int nfc_hci_register_device(struct nfc_hci_dev *hdev)
INIT_WORK(&hdev->msg_tx_work, nfc_hci_msg_tx_work);
- init_timer(&hdev->cmd_timer);
- hdev->cmd_timer.data = (unsigned long)hdev;
- hdev->cmd_timer.function = nfc_hci_cmd_timeout;
+ setup_timer(&hdev->cmd_timer, nfc_hci_cmd_timeout,
+ (unsigned long)hdev);
skb_queue_head_init(&hdev->rx_hcp_frags);
--
2.7.4
^ permalink raw reply related
* [PATCH 0/5] use setup_timer() helper function.
From: Allen Pais @ 2017-09-22 10:58 UTC (permalink / raw)
To: netdev; +Cc: davem, sameo, Allen Pais
This series uses setup_timer() helper function. The series
addresses the files under net/*.
Allen Pais (5):
net: nfc: hci: use setup_timer() helper.
net: nfc: hci: llc_shdlc: use setup_timer() helper.
net: af_packet: use setup_timer() helper.
net: nfc: core: use setup_timer() helper.
net: nfc: llcp_core: use setup_timer() helper.
net/nfc/core.c | 5 ++---
net/nfc/hci/core.c | 5 ++---
net/nfc/hci/llc_shdlc.c | 15 ++++++---------
net/nfc/llcp_core.c | 10 ++++------
net/packet/af_packet.c | 4 +---
5 files changed, 15 insertions(+), 24 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit
From: Julia Lawall @ 2017-09-22 10:54 UTC (permalink / raw)
To: Joe Perches
Cc: Colin Ian King, Stanislaw Gruszka, Kalle Valo, linux-wireless,
netdev, kernel-janitors, linux-kernel
In-Reply-To: <1506077181.12311.39.camel@perches.com>
[-- Attachment #1: Type: text/plain, Size: 3584 bytes --]
On Fri, 22 Sep 2017, Joe Perches wrote:
> On Fri, 2017-09-22 at 12:06 +0200, Julia Lawall wrote:
> >
> > On Fri, 22 Sep 2017, Colin Ian King wrote:
> >
> > > On 22/09/17 11:03, Joe Perches wrote:
> > > > On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> > > > >
> > > > > On Thu, 21 Sep 2017, Colin King wrote:
> > > > >
> > > > > > From: Colin Ian King <colin.king@canonical.com>
> > > > > >
> > > > > > Don't populate const array ac_to_fifo on the stack in an inlined
> > > > > > function, instead make it static. Makes the object code smaller
> > > > > > by over 800 bytes:
> > > > > >
> > > > > > text data bss dec hex filename
> > > > > > 159029 33154 1216 193399 2f377 4965-mac.o
> > > > > >
> > > > > > text data bss dec hex filename
> > > > > > 158122 33250 1216 192588 2f04c 4965-mac.o
> > > > > >
> > > > > > (gcc version 7.2.0 x86_64)
> > > > >
> > > > > Gcc 7 must be much more aggressive about inlining than gcc 4. Here is the
> > > > > change that I got on this file:
> > > > >
> > > > > text data bss dec hex filename
> > > > > - 76346 1494 152 77992 130a8 drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > > + 76298 1494 152 77944 13078 drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > > decrease of 48
> > > >
> > > > More likely different CONFIG options.
> > > >
> > > > e.g. allyesconfig vs defconfig with wireless
> > > >
> > >
> > > yup, I used allyesconfig
> >
> > Mine is also allyesconfig.
>
> Odd.
>
> Here is what I get: (Sorry, no gcc-7)
>
> gcc4: gcc-4.9 (Ubuntu 4.9.4-2ubuntu1) 4.9.4
> gcc5: gcc-5 (Ubuntu 5.4.1-8ubuntu1) 5.4.1 20170304
> gcc6: gcc-6 (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406
>
> $ size drivers/net/wireless/intel/iwlegacy/4965-mac.o*
> text data bss dec hex filename
> 118559 1766 152 120477 1d69d drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.new
> 118623 1766 152 120541 1d6dd drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.old
My gcc is 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.3)
You get a savings of 64 and I got a savings of 48, but it's not that big a
difference, compared to what the later versions give.
julia
> 51595 1156 0 52751 ce0f drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.new
> 51659 1156 0 52815 ce4f drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.old
> 141956 29790 1216 172962 2a3a2 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.new
> 142671 29702 1216 173589 2a615 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.old
> 51733 1156 0 52889 ce99 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.new
> 51813 1156 0 52969 cee9 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.old
> 152991 29790 1216 183997 2cebd drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.new
> 153722 29702 1216 184640 2d140 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.old
> 51813 1156 0 52969 cee9 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.new
> 51893 1156 0 53049 cf39 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.old
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit
From: Joe Perches @ 2017-09-22 10:46 UTC (permalink / raw)
To: Julia Lawall, Colin Ian King
Cc: Stanislaw Gruszka, Kalle Valo, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <alpine.DEB.2.20.1709221206050.3170@hadrien>
On Fri, 2017-09-22 at 12:06 +0200, Julia Lawall wrote:
>
> On Fri, 22 Sep 2017, Colin Ian King wrote:
>
> > On 22/09/17 11:03, Joe Perches wrote:
> > > On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> > > >
> > > > On Thu, 21 Sep 2017, Colin King wrote:
> > > >
> > > > > From: Colin Ian King <colin.king@canonical.com>
> > > > >
> > > > > Don't populate const array ac_to_fifo on the stack in an inlined
> > > > > function, instead make it static. Makes the object code smaller
> > > > > by over 800 bytes:
> > > > >
> > > > > text data bss dec hex filename
> > > > > 159029 33154 1216 193399 2f377 4965-mac.o
> > > > >
> > > > > text data bss dec hex filename
> > > > > 158122 33250 1216 192588 2f04c 4965-mac.o
> > > > >
> > > > > (gcc version 7.2.0 x86_64)
> > > >
> > > > Gcc 7 must be much more aggressive about inlining than gcc 4. Here is the
> > > > change that I got on this file:
> > > >
> > > > text data bss dec hex filename
> > > > - 76346 1494 152 77992 130a8 drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > + 76298 1494 152 77944 13078 drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > decrease of 48
> > >
> > > More likely different CONFIG options.
> > >
> > > e.g. allyesconfig vs defconfig with wireless
> > >
> >
> > yup, I used allyesconfig
>
> Mine is also allyesconfig.
Odd.
Here is what I get: (Sorry, no gcc-7)
gcc4: gcc-4.9 (Ubuntu 4.9.4-2ubuntu1) 4.9.4
gcc5: gcc-5 (Ubuntu 5.4.1-8ubuntu1) 5.4.1 20170304
gcc6: gcc-6 (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406
$ size drivers/net/wireless/intel/iwlegacy/4965-mac.o*
text data bss dec hex filename
118559 1766 152 120477 1d69d drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.new
118623 1766 152 120541 1d6dd drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.old
51595 1156 0 52751 ce0f drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.new
51659 1156 0 52815 ce4f drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.old
141956 29790 1216 172962 2a3a2 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.new
142671 29702 1216 173589 2a615 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.old
51733 1156 0 52889 ce99 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.new
51813 1156 0 52969 cee9 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.old
152991 29790 1216 183997 2cebd drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.new
153722 29702 1216 184640 2d140 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.old
51813 1156 0 52969 cee9 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.new
51893 1156 0 53049 cf39 drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.old
^ permalink raw reply
* Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit
From: Kalle Valo @ 2017-09-22 10:32 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Colin King, linux-wireless, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20170922095609.GA27551@redhat.com>
Stanislaw Gruszka <sgruszka@redhat.com> writes:
> On Thu, Sep 21, 2017 at 11:56:30PM +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Don't populate const array ac_to_fifo on the stack in an inlined
>> function, instead make it static. Makes the object code smaller
>> by over 800 bytes:
>>
>> text data bss dec hex filename
>> 159029 33154 1216 193399 2f377 4965-mac.o
>>
>> text data bss dec hex filename
>> 158122 33250 1216 192588 2f04c 4965-mac.o
>>
>> (gcc version 7.2.0 x86_64)
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> Content type information was added at the end of the topic, but
> I think Kalle can fix that when he will be committing the patch.
Yeah, I'll fix that when I commit this. But very good that you pointed
it out, I might miss stuff like this.
I'll also remove the "wireless:" prefix from the title.
--
Kalle Valo
^ 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