* Re: [Intel-wired-lan] [i40e] regression on TCP stream and TCP maerts, kernel-4.12.0-0.rc2
From: Duyck, Alexander H @ 2017-06-01 19:18 UTC (permalink / raw)
To: atomasov@redhat.com, alexander.duyck@gmail.com
Cc: osabart@redhat.com, netdev@vger.kernel.org, aokuliar@redhat.com,
intel-wired-lan@lists.osuosl.org, jhladky@redhat.com
In-Reply-To: <1496312080.3758.7.camel@redhat.com>
On Thu, 2017-06-01 at 12:14 +0200, Adrian Tomasov wrote:
> On Wed, 2017-05-31 at 14:42 -0700, Alexander Duyck wrote:
> >
> > On Wed, May 31, 2017 at 6:48 AM, Adrian Tomasov <atomasov@redhat.com>
> > wrote:
> > >
> > >
> > > On Tue, 2017-05-30 at 18:27 -0700, Alexander Duyck wrote:
> > > >
> > > >
> > > > On Tue, May 30, 2017 at 8:41 AM, Alexander Duyck
> > > > <alexander.duyck@gmail.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Tue, May 30, 2017 at 6:43 AM, Adam Okuliar <aokuliar@redhat.
> > > > > com>
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > we found regression on intel card(XL710) with i40e driver.
> > > > > > Regression is
> > > > > > about ~45%
> > > > > > on TCP_STREAM and TCP_MAERTS test for IPv4 and IPv6.
> > > > > > Regression
> > > > > > was first
> > > > > > visible in kernel-4.12.0-0.rc1.
> > > > > >
> > > > > > More details about results you can see in uploaded images in
> > > > > > bugzilla. [0]
> > > > > >
> > > > > >
> > > > > > [0] https://bugzilla.kernel.org/show_bug.cgi?id=195923
> > > > > >
> > > > > >
> > > > > > Best regards, / S pozdravom,
> > > > > >
> > > > > > Adrián Tomašov
> > > > > > Kernel Performance QE
> > > > > > atomasov@redhat.com
> > > > >
> > > > > I have added the i40e driver maintainer and the intel-wired-lan
> > > > > mailing list so that we can make are developers aware of the
> > > > > issue.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > - Alex
> > > >
> > > > Adam,
> > > >
> > > > We are having some issues trying to reproduce what you reported.
> > > >
> > > > Can you provide some additional data. Specifically we would be
> > > > looking
> > > > for an "ethtool -i", and an "ethtool -S" for the port before and
> > > > after
> > > > the test. If you can attach it to the bugzilla that would be
> > > > appreciated.
> > > >
> > > > Thanks.
> > > >
> > > > - Alex
> > >
> > > Hello Alex,
> > >
> > > requested files are updated in bugzilla.
> > >
> > > If you have any questions about testing feel free to ask.
> > >
> > >
> > > Best regards,
> > >
> > > Adrian
> >
> > So looking at the data I wonder if we don't have an MTU mismatch in
> > the network config. I notice the "after" has rx_length_errors being
> > reported. Recent changes made it so that i40e doesn't support jumbo
> > frames by default, whereas before we could. You might want to check
> > for that as that could cause the kind of performance issues you are
> > seeing.
> >
> > - Alex
>
> There isn't MTU mismatch. Traffic path is : server -> switch ->
> server.
>
>
> Output from switch:
>
> > show interfaces et-0/0/18
> Physical interface: et-0/0/18, Enabled, Physical link is Up
> Interface index: 644, SNMP ifIndex: 538
> Link-level type: Ethernet, MTU: 1514, Speed: 40Gbps, BPDU Error:
> None, MAC-REWRITE Error: None, Loopback: Disabled, Source filtering:
> Disabled, Flow control: Disabled, Media type: Fiber
> Device flags : Present Running
> Interface flags: SNMP-Traps Internal: 0x4000
> Link flags : None
> CoS queues : 12 supported, 12 maximum usable queues
> Current address: d4:04:ff:90:5a:4b, Hardware address:
> d4:04:ff:90:5a:4b
> Last flapped : 2017-06-01 10:09:32 CEST (01:21:29 ago)
> Input rate : 432 bps (0 pps)
> Output rate : 8336 bps (11 pps)
> Active alarms : None
> Active defects : None
> Interface transmit statistics: Disabled
>
> Logical interface et-0/0/18.0 (Index 552) (SNMP ifIndex 539)
> Flags: SNMP-Traps 0x24024000 Encapsulation: Ethernet-Bridge
> Input packets : 464041
> Output packets: 209210
> Protocol eth-switch, MTU: 1514
> Flags: Is-Primary, Trunk-Mode
>
>
> MTU is same for all et-0/0/x interfaces.
>
> - Adrian
One thing you might try try doing is toggling the legacy-rx flag using
the "ethtool --show-priv-flags/--set-priv-flags" command to see if that
has any impact. That will help to rule things out as the most
significant change I can think of is the recent update of the Rx path
to support XDP.
Also one other thing you might try would be to use a fixed interrupt
moderation rate by locking things down using "ethtool -C" to disable
adaptive interrupt moderation and lock the Rx usecs and Tx usecs at
some predefined values. I seem to recall there have been some interrupt
moderation changes made recently that might be impacting the
performance.
Beyond that is there any chance you would be able to bisect the issue?
Unfortunately we haven't be able to reproduce it internally so anything
that would help us to narrow down the problem would be useful.
Thanks.
- Alex
^ permalink raw reply
* Re: [PATCH V4] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"
From: Julian Anastasov @ 2017-06-01 19:34 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: netdev, davem, stephen
In-Reply-To: <1496280282-84895-1-git-send-email-sowmini.varadhan@oracle.com>
Hello,
On Wed, 31 May 2017, Sowmini Varadhan wrote:
> The command
> # arp -s 62.2.0.1 a:b:c:d:e:f dev eth2
> adds an entry like the following (listed by "arp -an")
> ? (62.2.0.1) at 0a:0b:0c:0d:0e:0f [ether] PERM on eth2
> but the symmetric deletion command
> # arp -i eth2 -d 62.2.0.1
> does not remove the PERM entry from the table, and instead leaves behind
> ? (62.2.0.1) at <incomplete> on eth2
>
> The reason is that there is a refcnt of 1 for the arp_tbl itself
> (neigh_alloc starts off the entry with a refcnt of 1), thus
> the neigh_release() call from arp_invalidate() will (at best) just
> decrement the ref to 1, but will never actually free it from the
> table.
>
> To fix this, we need to do something like neigh_forced_gc: if
> the refcnt is 1 (i.e., on the table's ref), remove the entry from
> the table and free it. This patch refactors and shares common code
> between neigh_forced_gc and the newly added neigh_remove_one.
>
> A similar issue exists for IPv6 Neighbor Cache entries, and is fixed
> in a similar manner by this patch.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Change looks ok to me but with some non-fatal
warnings, see below.
Reviewed-by: Julian Anastasov <ja@ssi.bg>
> ---
> v2: kbuild-test-robot compile error
> v3: do not export_symbol neigh_remove_one (David Miller comment)
> v4: rearrange locking for tbl->lock (Julian Anastasov comment)
>
> include/net/neighbour.h | 1 +
> net/core/neighbour.c | 61 ++++++++++++++++++++++++++++++++++++++--------
> net/ipv4/arp.c | 4 +++
> 3 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index e4dd3a2..639b675 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -317,6 +317,7 @@ struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
> int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, u32 flags,
> u32 nlmsg_pid);
> void __neigh_set_probe_once(struct neighbour *neigh);
> +bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl);
> void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
> int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
> int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb);
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index d274f81..b473f9f 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
...
> +bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl)
> +{
> + struct neigh_hash_table *nht;
> + void *pkey = ndel->primary_key;
> + u32 hash_val;
> + struct neighbour *n;
> + struct neighbour __rcu **np;
> + bool retval = false;
> +
> + nht = rcu_dereference_protected(tbl->nht,
> + lockdep_is_held(&tbl->lock));
> + hash_val = tbl->hash(pkey, ndel->dev, nht->hash_rnd);
> + hash_val = hash_val >> (32 - nht->hash_shift);
> +
> + np = &nht->hash_buckets[hash_val];
> + while ((n = rcu_dereference_protected(*np,
> + lockdep_is_held(&tbl->lock))) != NULL) {
checkpatch shows some warnings:
scripts/checkpatch.pl --strict /tmp/file.patch
> + if (n == ndel) {
> + retval = neigh_del(n, 0, np, tbl);
> + return retval;
In case there is another patch version,
the retval can be removed:
if (n == ndel)
return neigh_del(n, 0, np, tbl);
> + }
> + np = &n->next;
> + }
> + return retval;
return false;
> +}
> +
> static int neigh_forced_gc(struct neigh_table *tbl)
> {
> int shrunk = 0;
> @@ -140,19 +185,10 @@ static int neigh_forced_gc(struct neigh_table *tbl)
> * - nobody refers to it.
> * - it is not permanent
> */
> - write_lock(&n->lock);
> - if (atomic_read(&n->refcnt) == 1 &&
> - !(n->nud_state & NUD_PERMANENT)) {
> - rcu_assign_pointer(*np,
> - rcu_dereference_protected(n->next,
> - lockdep_is_held(&tbl->lock)));
> - n->dead = 1;
> - shrunk = 1;
> - write_unlock(&n->lock);
> - neigh_cleanup_and_release(n);
> + if (neigh_del(n, NUD_PERMANENT, np, tbl)) {
> + shrunk = 1;
> continue;
> }
> - write_unlock(&n->lock);
> np = &n->next;
> }
> }
> @@ -1649,7 +1685,10 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
> NEIGH_UPDATE_F_OVERRIDE |
> NEIGH_UPDATE_F_ADMIN,
> NETLINK_CB(skb).portid);
> + write_lock_bh(&tbl->lock);
> neigh_release(neigh);
> + neigh_remove_one(neigh, tbl);
Looks like we can also call neigh_remove_one only when !err.
But this is some corner case when n->dead is set by GC and entry
was unlinked, neigh_remove_one simply will not find it in the list,
so it is not fatal to call neigh_remove_one unconditionally.
> + write_unlock_bh(&tbl->lock);
>
> out:
> return err;
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index e9f3386..a651c53 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -1113,13 +1113,17 @@ static int arp_invalidate(struct net_device *dev, __be32 ip)
> {
> struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
> int err = -ENXIO;
> + struct neigh_table *tbl = &arp_tbl;
>
> if (neigh) {
> if (neigh->nud_state & ~NUD_NOARP)
> err = neigh_update(neigh, NULL, NUD_FAILED,
> NEIGH_UPDATE_F_OVERRIDE|
> NEIGH_UPDATE_F_ADMIN, 0);
> + write_lock_bh(&tbl->lock);
> neigh_release(neigh);
> + neigh_remove_one(neigh, tbl);
Here the same race with GC already assigned
neigh->dead to 1 is possible but it is more tricky to catch
that exactly neigh_update() has failed. So, may be better to
call neigh_remove_one like now.
> + write_unlock_bh(&tbl->lock);
> }
>
> return err;
> --
> 1.7.1
Regards
^ permalink raw reply
* Re: [PATCH net-next] ipv4: route: restore skb_dst_set in inet_rtm_getroute
From: John Fastabend @ 2017-06-01 19:43 UTC (permalink / raw)
To: David Miller, roopa; +Cc: netdev, dsahern
In-Reply-To: <20170601.113429.1492311634394467957.davem@davemloft.net>
On 06/01/2017 08:34 AM, David Miller wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> Date: Wed, 31 May 2017 22:53:25 -0700
>
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> recent updates to inet_rtm_getroute dropped skb_dst_set in
>> inet_rtm_getroute. This patch restores it because it is
>> needed to release the dst correctly.
>>
>> Fixes: 3765d35ed8b9 ("net: ipv4: Convert inet_rtm_getroute to rcu versions of route lookup")
>> Reported-by: John Fastabend <john.fastabend@gmail.com>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Applied.
>
Thanks Roopa this fixes the regression on my systems.
.John
^ permalink raw reply
* Re: [PATCH net] net: bridge: start hello timer only if device is up
From: Sebastian Ott @ 2017-06-01 19:44 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, davem, roopa, Xin Long, Ivan Vecera
In-Reply-To: <1496329675-11254-1-git-send-email-nikolay@cumulusnetworks.com>
On Thu, 1 Jun 2017, Nikolay Aleksandrov wrote:
> When the transition of NO_STP -> KERNEL_STP was fixed by always calling
> mod_timer in br_stp_start, it introduced a new regression which causes
> the timer to be armed even when the bridge is down, and since we stop
> the timers in its ndo_stop() function, they never get disabled if the
> device is destroyed before it's upped.
>
> To reproduce:
> $ while :; do ip l add br0 type bridge hello_time 100; brctl stp br0 on;
> ip l del br0; done;
>
> CC: Xin Long <lucien.xin@gmail.com>
> CC: Ivan Vecera <cera@cera.cz>
> CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
> Reported-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
> Fixes: 6d18c732b95c ("bridge: start hello_timer when enabling KERNEL_STP in br_stp_start")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> Sebastian it'd be great if you can test the patch as well.
I did and can confirm that it fixes the problem.
Thanks,
Sebastian
^ permalink raw reply
* Re: [PATCH V4] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"
From: Sowmini Varadhan @ 2017-06-01 19:45 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev, davem, stephen
In-Reply-To: <alpine.LFD.2.20.1706012201100.3101@ja.home.ssi.bg>
On (06/01/17 22:34), Julian Anastasov wrote:
> > + np = &nht->hash_buckets[hash_val];
> > + while ((n = rcu_dereference_protected(*np,
> > + lockdep_is_held(&tbl->lock))) != NULL) {
>
> checkpatch shows some warnings:
>
> scripts/checkpatch.pl --strict /tmp/file.patch
Yes, checkpatch complained about
"CHECK: Alignment should match open parenthesis"
but trying to meet that requirement (without exceeding the 80 char limit)
would need additional variables, and I noticed that there are
other places in the code (e.g., neigh_forced_gc()) where the alignment
prescription is not observed, so I let things follow existing style..
[ In neigh_remove_one()]
> In case there is another patch version,
> the retval can be removed:
Let me see if there are additional review comments, and I can update
with the retval removed.
Thanks much for the review!
> Looks like we can also call neigh_remove_one only when !err.
> But this is some corner case when n->dead is set by GC and entry
> was unlinked, neigh_remove_one simply will not find it in the list,
> so it is not fatal to call neigh_remove_one unconditionally.
> > @@ -1113,13 +1113,17 @@ static int arp_invalidate(struct net_device *dev, __be32 ip)
:
> Here the same race with GC already assigned
> neigh->dead to 1 is possible but it is more tricky to catch
> that exactly neigh_update() has failed. So, may be better to
> call neigh_remove_one like now.
^ permalink raw reply
* Re: commit f5f99309 (sock: do not set sk_err in sock_dequeue_err_skb) has broken ping
From: Cyril Hrubis @ 2017-06-01 20:03 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David S. Miller, Shmulik Ladkani, Marcelo Ricardo Leitner,
Pravin B Shelar, Eric Dumazet, WANG Cong, Yaogong Wang,
Steffen Klassert, Al Viro, netdev, linux-kernel
In-Reply-To: <CACSApvb_CtFEenaSGcpUk6yb8O3X95MAULkJ-+T1hscCE_O_jg@mail.gmail.com>
Hi!
> Thank you very much! I have a wild guess that, when we
> have a train of skbs on the error queue starting from a local error,
> we will see this issue.
>
> Ping (without my patch) considers EAGAIN on a normal read as an
> indication that there is nothing on the error queue, but that's a
> flawed assumption.
>
> Would you mind trying another shot in the darkness please? Thanks!
This patch seems to fix the issue, I've tried several times and poll()
just timeouts, haven't seen a single POLLERR in the ping strace.
You can add my Tested-by: for this patch as well :-).
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply
* [PATCH net-next v3 0/5] net: dsa: tagger simplification
From: Vivien Didelot @ 2017-06-01 20:07 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
This series clarifies the hot path, removes the labels in tagging
implementations, and frees the original SKB in the xmit caller.
Changes in v3:
- drop removal of usused rcv arguments because they will be used later
- include the new ksz tagging implementation
- add reviewers' tags
Changes in v2:
- do not remove tagger function copies
- document hot path requirements
- make netdev_uses_dsa simpler
- add reviewers' tags
Vivien Didelot (5):
net: dsa: comment hot path requirements
net: dsa: do not cast dst
net: dsa: remove dsa_uses_tagged_protocol
net: dsa: remove out_drop label in taggers rcv
net: dsa: factor skb freeing on xmit
include/net/dsa.h | 11 ++++-------
net/dsa/dsa2.c | 2 +-
net/dsa/dsa_priv.h | 1 +
net/dsa/legacy.c | 2 +-
net/dsa/slave.c | 8 ++++++--
net/dsa/tag_brcm.c | 17 +++++------------
net/dsa/tag_dsa.c | 21 +++++++--------------
net/dsa/tag_edsa.c | 21 +++++++--------------
net/dsa/tag_ksz.c | 4 +---
net/dsa/tag_lan9303.c | 5 +----
net/dsa/tag_mtk.c | 15 ++++-----------
net/dsa/tag_qca.c | 17 +++++------------
net/dsa/tag_trailer.c | 13 ++++---------
13 files changed, 47 insertions(+), 90 deletions(-)
--
2.13.0
^ permalink raw reply
* [PATCH net-next v3 1/5] net: dsa: comment hot path requirements
From: Vivien Didelot @ 2017-06-01 20:07 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170601200715.8438-1-vivien.didelot@savoirfairelinux.com>
The DSA layer uses inline helpers and copy of the tagging functions for
faster access in hot path. Add comments to detail that.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
include/net/dsa.h | 3 +++
net/dsa/dsa_priv.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7de1234ba136..18ca0a935c96 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -127,6 +127,8 @@ struct dsa_switch_tree {
* protocol to use.
*/
struct net_device *master_netdev;
+
+ /* Copy of tag_ops->rcv for faster access in hot path */
struct sk_buff * (*rcv)(struct sk_buff *skb,
struct net_device *dev,
struct packet_type *pt,
@@ -465,6 +467,7 @@ struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
struct net_device *dsa_dev_to_net_device(struct device *dev);
+/* Keep inline for faster access in hot path */
static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
{
return dst->rcv != NULL;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7459d5735d8b..db2a7b9edfb8 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -73,6 +73,7 @@ struct dsa_device_ops {
};
struct dsa_slave_priv {
+ /* Copy of dp->ds->dst->tag_ops->xmit for faster access in hot path */
struct sk_buff * (*xmit)(struct sk_buff *skb,
struct net_device *dev);
--
2.13.0
^ permalink raw reply related
* [PATCH net-next v3 2/5] net: dsa: do not cast dst
From: Vivien Didelot @ 2017-06-01 20:07 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170601200715.8438-1-vivien.didelot@savoirfairelinux.com>
dsa_ptr is not a void pointer anymore since Nov 2011, as of cf50dcc24f82
("dsa: Change dsa_uses_{dsa, trailer}_tags() into inline functions"),
but an explicit dsa_switch_tree pointer, thus remove the (void *) cast.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa2.c | 2 +-
net/dsa/legacy.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index c0a4576db4a2..21b44a9828f6 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -454,7 +454,7 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
* sent to the tag format's receive function.
*/
wmb();
- dst->master_netdev->dsa_ptr = (void *)dst;
+ dst->master_netdev->dsa_ptr = dst;
dst->applied = true;
return 0;
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index ac4379b8d7ac..d70a1a788d17 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -651,7 +651,7 @@ static int dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
* sent to the tag format's receive function.
*/
wmb();
- dev->dsa_ptr = (void *)dst;
+ dev->dsa_ptr = dst;
return 0;
}
--
2.13.0
^ permalink raw reply related
* [PATCH net-next v3 3/5] net: dsa: remove dsa_uses_tagged_protocol
From: Vivien Didelot @ 2017-06-01 20:07 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170601200715.8438-1-vivien.didelot@savoirfairelinux.com>
Since dev->dsa_ptr is a pointer to a dsa_switch_tree, there is no need
to have another inline helper just to check rcv.
Remove dsa_uses_tagged_protocol and check dsa_ptr && dsa_ptr->rcv
together at the same time.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
include/net/dsa.h | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 18ca0a935c96..448d8bc77707 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -468,16 +468,10 @@ struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
struct net_device *dsa_dev_to_net_device(struct device *dev);
/* Keep inline for faster access in hot path */
-static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
-{
- return dst->rcv != NULL;
-}
-
static inline bool netdev_uses_dsa(struct net_device *dev)
{
#if IS_ENABLED(CONFIG_NET_DSA)
- if (dev->dsa_ptr != NULL)
- return dsa_uses_tagged_protocol(dev->dsa_ptr);
+ return dev->dsa_ptr && dev->dsa_ptr->rcv;
#endif
return false;
}
--
2.13.0
^ permalink raw reply related
* [PATCH net-next v3 4/5] net: dsa: remove out_drop label in taggers rcv
From: Vivien Didelot @ 2017-06-01 20:07 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170601200715.8438-1-vivien.didelot@savoirfairelinux.com>
Many rcv functions from net/dsa/tag_*.c have a useless out_drop goto
label which simply returns NULL. Kill it in favor of the obvious.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/tag_brcm.c | 11 ++++-------
net/dsa/tag_dsa.c | 13 +++++--------
net/dsa/tag_edsa.c | 13 +++++--------
net/dsa/tag_mtk.c | 9 +++------
net/dsa/tag_qca.c | 11 ++++-------
net/dsa/tag_trailer.c | 9 +++------
6 files changed, 24 insertions(+), 42 deletions(-)
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 9f204f18ada3..635ecb6781e4 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -104,27 +104,27 @@ static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
ds = dst->cpu_dp->ds;
if (unlikely(!pskb_may_pull(skb, BRCM_TAG_LEN)))
- goto out_drop;
+ return NULL;
/* skb->data points to the EtherType, the tag is right before it */
brcm_tag = skb->data - 2;
/* The opcode should never be different than 0b000 */
if (unlikely((brcm_tag[0] >> BRCM_OPCODE_SHIFT) & BRCM_OPCODE_MASK))
- goto out_drop;
+ return NULL;
/* We should never see a reserved reason code without knowing how to
* handle it
*/
if (unlikely(brcm_tag[2] & BRCM_EG_RC_RSVD))
- goto out_drop;
+ return NULL;
/* Locate which port this is coming from */
source_port = brcm_tag[3] & BRCM_EG_PID_MASK;
/* Validate port against switch setup, either the port is totally */
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
/* Remove Broadcom tag and update checksum */
skb_pull_rcsum(skb, BRCM_TAG_LEN);
@@ -137,9 +137,6 @@ static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops brcm_netdev_ops = {
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 3b62a57956a3..089c99c8ed51 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -79,7 +79,7 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev,
int source_port;
if (unlikely(!pskb_may_pull(skb, DSA_HLEN)))
- goto out_drop;
+ return NULL;
/*
* The ethertype field is part of the DSA header.
@@ -90,7 +90,7 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev,
* Check that frame type is either TO_CPU or FORWARD.
*/
if ((dsa_header[0] & 0xc0) != 0x00 && (dsa_header[0] & 0xc0) != 0xc0)
- goto out_drop;
+ return NULL;
/*
* Determine source device and port.
@@ -103,14 +103,14 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev,
* port is a registered DSA port.
*/
if (source_device >= DSA_MAX_SWITCHES)
- goto out_drop;
+ return NULL;
ds = dst->ds[source_device];
if (!ds)
- goto out_drop;
+ return NULL;
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
/*
* Convert the DSA header to an 802.1q header if the 'tagged'
@@ -161,9 +161,6 @@ static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev,
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops dsa_netdev_ops = {
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index f95cafd05702..a7eed1d43d80 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -92,7 +92,7 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev,
int source_port;
if (unlikely(!pskb_may_pull(skb, EDSA_HLEN)))
- goto out_drop;
+ return NULL;
/*
* Skip the two null bytes after the ethertype.
@@ -103,7 +103,7 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev,
* Check that frame type is either TO_CPU or FORWARD.
*/
if ((edsa_header[0] & 0xc0) != 0x00 && (edsa_header[0] & 0xc0) != 0xc0)
- goto out_drop;
+ return NULL;
/*
* Determine source device and port.
@@ -116,14 +116,14 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev,
* port is a registered DSA port.
*/
if (source_device >= DSA_MAX_SWITCHES)
- goto out_drop;
+ return NULL;
ds = dst->ds[source_device];
if (!ds)
- goto out_drop;
+ return NULL;
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
/*
* If the 'tagged' bit is set, convert the DSA tag to a 802.1q
@@ -180,9 +180,6 @@ static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev,
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops edsa_netdev_ops = {
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index d1258e84cd71..4b4aaf1574aa 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -57,7 +57,7 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
__be16 *phdr, hdr;
if (unlikely(!pskb_may_pull(skb, MTK_HDR_LEN)))
- goto out_drop;
+ return NULL;
/* The MTK header is added by the switch between src addr
* and ethertype at this point, skb->data points to 2 bytes
@@ -79,19 +79,16 @@ static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
*/
ds = dst->ds[0];
if (!ds)
- goto out_drop;
+ return NULL;
/* Get source port information */
port = (hdr & MTK_HDR_RECV_SOURCE_PORT_MASK);
if (!ds->ports[port].netdev)
- goto out_drop;
+ return NULL;
skb->dev = ds->ports[port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops mtk_netdev_ops = {
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 2451007699b7..44f545d2761a 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -77,7 +77,7 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
__be16 *phdr, hdr;
if (unlikely(!pskb_may_pull(skb, QCA_HDR_LEN)))
- goto out_drop;
+ return NULL;
/* The QCA header is added by the switch between src addr and Ethertype
* At this point, skb->data points to ethertype so header should be
@@ -89,7 +89,7 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
/* Make sure the version is correct */
ver = (hdr & QCA_HDR_RECV_VERSION_MASK) >> QCA_HDR_RECV_VERSION_S;
if (unlikely(ver != QCA_HDR_VERSION))
- goto out_drop;
+ return NULL;
/* Remove QCA tag and recalculate checksum */
skb_pull_rcsum(skb, QCA_HDR_LEN);
@@ -101,20 +101,17 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
*/
ds = dst->cpu_dp->ds;
if (!ds)
- goto out_drop;
+ return NULL;
/* Get source port information */
port = (hdr & QCA_HDR_RECV_SOURCE_PORT_MASK);
if (!ds->ports[port].netdev)
- goto out_drop;
+ return NULL;
/* Update skb & forward the frame accordingly */
skb->dev = ds->ports[port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops qca_netdev_ops = {
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 7488ab2932ab..ec729c0ef390 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -70,25 +70,22 @@ static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev,
ds = dst->cpu_dp->ds;
if (skb_linearize(skb))
- goto out_drop;
+ return NULL;
trailer = skb_tail_pointer(skb) - 4;
if (trailer[0] != 0x80 || (trailer[1] & 0xf8) != 0x00 ||
(trailer[2] & 0xef) != 0x00 || trailer[3] != 0x00)
- goto out_drop;
+ return NULL;
source_port = trailer[1] & 7;
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
- goto out_drop;
+ return NULL;
pskb_trim_rcsum(skb, skb->len - 4);
skb->dev = ds->ports[source_port].netdev;
return skb;
-
-out_drop:
- return NULL;
}
const struct dsa_device_ops trailer_netdev_ops = {
--
2.13.0
^ permalink raw reply related
* [PATCH net-next v3 5/5] net: dsa: factor skb freeing on xmit
From: Vivien Didelot @ 2017-06-01 20:07 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170601200715.8438-1-vivien.didelot@savoirfairelinux.com>
As of a86d8becc3f0 ("net: dsa: Factor bottom tag receive functions"),
the rcv caller frees the original SKB in case or error.
Be symmetric with that and make the xmit caller do the same.
At the same time, fix the checkpatch NULL comparison check:
CHECK: Comparison to NULL could be written "!nskb"
#208: FILE: net/dsa/tag_trailer.c:35:
+ if (nskb == NULL)
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/slave.c | 8 ++++++--
net/dsa/tag_brcm.c | 6 +-----
net/dsa/tag_dsa.c | 8 ++------
net/dsa/tag_edsa.c | 8 ++------
net/dsa/tag_ksz.c | 4 +---
net/dsa/tag_lan9303.c | 5 +----
net/dsa/tag_mtk.c | 6 +-----
net/dsa/tag_qca.c | 6 +-----
net/dsa/tag_trailer.c | 4 +---
9 files changed, 16 insertions(+), 39 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 0442b6bf52fa..1cfdb31a2f44 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -357,10 +357,14 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
- /* Transmit function may have to reallocate the original SKB */
+ /* Transmit function may have to reallocate the original SKB,
+ * in which case it must have freed it. Only free it here on error.
+ */
nskb = p->xmit(skb, dev);
- if (!nskb)
+ if (!nskb) {
+ kfree_skb(skb);
return NETDEV_TX_OK;
+ }
/* SKB for netpoll still need to be mangled with the protocol-specific
* tag to be successfully transmitted
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 635ecb6781e4..c03860907f28 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -65,7 +65,7 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
u8 *brcm_tag;
if (skb_cow_head(skb, BRCM_TAG_LEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, BRCM_TAG_LEN);
@@ -86,10 +86,6 @@ static struct sk_buff *brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev
brcm_tag[3] = (1 << p->dp->index) & BRCM_IG_DSTMAP1_MASK;
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *brcm_tag_rcv(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 089c99c8ed51..12867a4b458f 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -28,7 +28,7 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
*/
if (skb->protocol == htons(ETH_P_8021Q)) {
if (skb_cow_head(skb, 0) < 0)
- goto out_free;
+ return NULL;
/*
* Construct tagged FROM_CPU DSA tag from 802.1q tag.
@@ -46,7 +46,7 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
} else {
if (skb_cow_head(skb, DSA_HLEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, DSA_HLEN);
memmove(skb->data, skb->data + DSA_HLEN, 2 * ETH_ALEN);
@@ -62,10 +62,6 @@ static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *dsa_rcv(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index a7eed1d43d80..67a9d26f9075 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -30,7 +30,7 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
*/
if (skb->protocol == htons(ETH_P_8021Q)) {
if (skb_cow_head(skb, DSA_HLEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, DSA_HLEN);
memmove(skb->data, skb->data + DSA_HLEN, 2 * ETH_ALEN);
@@ -55,7 +55,7 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
} else {
if (skb_cow_head(skb, EDSA_HLEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, EDSA_HLEN);
memmove(skb->data, skb->data + EDSA_HLEN, 2 * ETH_ALEN);
@@ -75,10 +75,6 @@ static struct sk_buff *edsa_xmit(struct sk_buff *skb, struct net_device *dev)
}
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *edsa_rcv(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index dfcd2fff5b13..b94a334a1d02 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -46,10 +46,8 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
} else {
nskb = alloc_skb(NET_IP_ALIGN + skb->len +
padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
- if (!nskb) {
- kfree_skb(skb);
+ if (!nskb)
return NULL;
- }
skb_reserve(nskb, NET_IP_ALIGN);
skb_reset_mac_header(nskb);
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index afd59330b5f1..247774d149f9 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -52,7 +52,7 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
if (skb_cow_head(skb, LAN9303_TAG_LEN) < 0) {
dev_dbg(&dev->dev,
"Cannot make room for the special tag. Dropping packet\n");
- goto out_free;
+ return NULL;
}
/* provide 'LAN9303_TAG_LEN' bytes additional space */
@@ -66,9 +66,6 @@ static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
lan9303_tag[1] = htons(p->dp->index | BIT(4));
return skb;
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
index 4b4aaf1574aa..2f32b7ea3365 100644
--- a/net/dsa/tag_mtk.c
+++ b/net/dsa/tag_mtk.c
@@ -27,7 +27,7 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
u8 *mtk_tag;
if (skb_cow_head(skb, MTK_HDR_LEN) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, MTK_HDR_LEN);
@@ -41,10 +41,6 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
mtk_tag[3] = 0;
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c
index 44f545d2761a..4f43cf0b4eff 100644
--- a/net/dsa/tag_qca.c
+++ b/net/dsa/tag_qca.c
@@ -45,7 +45,7 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
dev->stats.tx_bytes += skb->len;
if (skb_cow_head(skb, 0) < 0)
- goto out_free;
+ return NULL;
skb_push(skb, QCA_HDR_LEN);
@@ -60,10 +60,6 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev)
*phdr = htons(hdr);
return skb;
-
-out_free:
- kfree_skb(skb);
- return NULL;
}
static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev,
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index ec729c0ef390..b4f6db094409 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -32,10 +32,8 @@ static struct sk_buff *trailer_xmit(struct sk_buff *skb, struct net_device *dev)
padlen = 60 - skb->len;
nskb = alloc_skb(NET_IP_ALIGN + skb->len + padlen + 4, GFP_ATOMIC);
- if (nskb == NULL) {
- kfree_skb(skb);
+ if (!nskb)
return NULL;
- }
skb_reserve(nskb, NET_IP_ALIGN);
skb_reset_mac_header(nskb);
--
2.13.0
^ permalink raw reply related
* Re: [PATCH net-next v3 5/5] net: dsa: factor skb freeing on xmit
From: Florian Fainelli @ 2017-06-01 20:14 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170601200715.8438-6-vivien.didelot@savoirfairelinux.com>
On 06/01/2017 01:07 PM, Vivien Didelot wrote:
> As of a86d8becc3f0 ("net: dsa: Factor bottom tag receive functions"),
> the rcv caller frees the original SKB in case or error.
>
> Be symmetric with that and make the xmit caller do the same.
>
> At the same time, fix the checkpatch NULL comparison check:
>
> CHECK: Comparison to NULL could be written "!nskb"
> #208: FILE: net/dsa/tag_trailer.c:35:
> + if (nskb == NULL)
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
I must have not read the patch correctly the first time, because my
objection no longer stands, this looks much cleaner now, thanks!
--
Florian
^ permalink raw reply
* [PATCH net v2] ibmvnic: Remove module author mailing address
From: Thomas Falcon @ 2017-06-01 20:32 UTC (permalink / raw)
To: netdev; +Cc: davem, nfont, Thomas Falcon, John Allen
The original author left the project and so far has not
responded to emails sent to the listed address.
Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
v2: only remove invalid email address
---
drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 4f2d329..a93757c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -81,7 +81,7 @@
static const char ibmvnic_driver_name[] = "ibmvnic";
static const char ibmvnic_driver_string[] = "IBM System i/p Virtual NIC Driver";
-MODULE_AUTHOR("Santiago Leon <santi_leon@yahoo.com>");
+MODULE_AUTHOR("Santiago Leon");
MODULE_DESCRIPTION("IBM System i/p Virtual NIC Driver");
MODULE_LICENSE("GPL");
MODULE_VERSION(IBMVNIC_DRIVER_VERSION);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 2/3] udp: avoid a cache miss on dequeue
From: Paolo Abeni @ 2017-06-01 20:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, netdev, David S. Miller
In-Reply-To: <CANn89iKznSPH9VogQEPSw4bLL8F_L_qj2ZY2M==VZ-O1u0xqiA@mail.gmail.com>
On Thu, 2017-06-01 at 09:40 -0700, Eric Dumazet wrote:
> On Thu, Jun 1, 2017 at 9:21 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>
> > I'm sorry, I do not follow. I'm concerned about the secpath field (skb-
> > > sp), which is the only one that can be not NULL in
> >
> > __udp_queue_rcv_skb().
> >
> > If the secpath is not NULL, calling there secpath_reset() (or the to-
> > be-introduced skb_reset_head_state()), we will properly release it and
> > we will clear the field, too.
> >
> > Calling skb_release_head_state() in the same scenario, we release the
> > secpath, but we don't clear it. So if the packet is later dropped we
> > will get a double free, unless we add and use a specialized a
> > free_stateless_skb(), too.
>
> Then simply use secpath_reset() instead of secpath_put() from
> skb_release_head_state()
>
> Clearly having these subtle differences bring confusion, for very little gain.
>
> secpath_put() should be removed. Most of its callers simply set
> skb->sp back to NULL anyway.
To make the code robust we would have to NULL all the other fields
(nfct, nf_bridge, destructor, sk) that are currently not cleared in
skb_release_head_state(), elsewhere if one day, after some change, any
that fields become non-NULL in this code path we risk a double-free
after skb_release_head_state(), even if the code looks safe.
Will that be a little too invasive for this small use-case? Can't we
prefer a new helper or simply a secpath_reset() plus some appropriate
comments?
Thanks,
Paolo
^ permalink raw reply
* Re: [PATCH net] netlink: don't send unknown nsid
From: Nicolas Dichtel @ 2017-06-01 20:42 UTC (permalink / raw)
To: Flavio Leitner; +Cc: davem, netdev
In-Reply-To: <20170601170246.GF2673@x240.lan>
Le 01/06/2017 à 19:02, Flavio Leitner a écrit :
> On Thu, Jun 01, 2017 at 10:00:07AM +0200, Nicolas Dichtel wrote:
>> The NETLINK_F_LISTEN_ALL_NSID otion enables to listen all netns that have a
>> nsid assigned into the netns where the netlink socket is opened.
>> The nsid is sent as metadata to userland, but the existence of this nsid is
>> checked only for netns that are different from the socket netns. Thus, if
>> no nsid is assigned to the socket netns, NETNSA_NSID_NOT_ASSIGNED is
>> reported to the userland. This value is confusing and useless.
>> After this patch, only valid nsid are sent to userland.
>>
>> Reported-by: Flavio Leitner <fbl@sysclose.org>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>> net/netlink/af_netlink.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
>> index ee841f00a6ec..7586d446d7dc 100644
>> --- a/net/netlink/af_netlink.c
>> +++ b/net/netlink/af_netlink.c
>> @@ -62,6 +62,7 @@
>> #include <asm/cacheflush.h>
>> #include <linux/hash.h>
>> #include <linux/genetlink.h>
>> +#include <linux/net_namespace.h>
>>
>> #include <net/net_namespace.h>
>> #include <net/sock.h>
>> @@ -1415,7 +1416,8 @@ static void do_one_broadcast(struct sock *sk,
>> goto out;
>> }
>> NETLINK_CB(p->skb2).nsid = peernet2id(sock_net(sk), p->net);
>> - NETLINK_CB(p->skb2).nsid_is_set = true;
>> + if (NETLINK_CB(p->skb2).nsid != NETNSA_NSID_NOT_ASSIGNED)
>> + NETLINK_CB(p->skb2).nsid_is_set = true;
>> val = netlink_broadcast_deliver(sk, p->skb2);
>> if (val < 0) {
>> netlink_overrun(sk);
>
> If the assumption is that nsid allocation can never fail or that if it
> does, we can't report to userspace, then the patch is good, but it
> doesn't sound like a good long term solution.
>
> Let's consider that the allocation of an id fails for whatever reason.
> I think that should be reported to userspace to allow it to retry, or
> do something else to handle this situation properly. Not sending
> anything means that it's in the same netns as the old kernels did,
> which is incorrect.
This is correct, because if nsid allocation fails, no netlink messages from this
netns are sent to userspace (the check is done at the beginning of
do_one_broadcast). The only netns allowed to send netlink messages to userspace
without nsid is the netns of the socket.
>
> On the other hand, with the original patch, if the socket and the
> device are in the same netns, we don't need to report any ID. Previous
> kernels did that, so we are not breaking anything. When the netns
> differs, then we either should report the real ID or an error.
>
I don't understand. With or without my last patch, the kernel sends netlink
messages of other netns than the netns where the socket is opened, only if an
nsid is assigned.
Nicolas
ps: I won't be able to read my emails before monday ;-)
^ permalink raw reply
* Re: [PATCH net-next 2/3] udp: avoid a cache miss on dequeue
From: Eric Dumazet @ 2017-06-01 20:50 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Eric Dumazet, netdev, David S. Miller
In-Reply-To: <1496349346.2798.1.camel@redhat.com>
On Thu, 2017-06-01 at 22:35 +0200, Paolo Abeni wrote:
> To make the code robust we would have to NULL all the other fields
> (nfct, nf_bridge, destructor, sk) that are currently not cleared in
> skb_release_head_state(), elsewhere if one day, after some change, any
> that fields become non-NULL in this code path we risk a double-free
> after skb_release_head_state(), even if the code looks safe.
Well, one can not predict future bugs, and we should not add code just
trying to prevent future bugs.
>
> Will that be a little too invasive for this small use-case? Can't we
> prefer a new helper or simply a secpath_reset() plus some appropriate
> comments?
Please reuse existing functions, and amend them if needed.
Fact that we mention secpath_put() or secpath_reset() in an UDP patch
should really tell us something is wrong. This is becoming a maintenance
burden.
^ permalink raw reply
* Re: [PATCH net-next 1/9] sched: add helper for updating statistics on all actions
From: Or Gerlitz @ 2017-06-01 20:54 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Linux Netdev List, oss-drivers, Jiri Pirko, Jamal Hadi Salim,
Cong Wang, Saeed Mahameed, Or Gerlitz
In-Reply-To: <20170531150651.16873-2-jakub.kicinski@netronome.com>
On Wed, May 31, 2017 at 6:06 PM, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
> Forgetting to disable preemption around tcf_action_stats_update()
> seems to be a common mistake. Add a helper function for updating
> stats on all actions of a filter.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> CC: Jiri Pirko <jiri@mellanox.com>
> CC: Jamal Hadi Salim <jhs@mojatatu.com>
> CC: Cong Wang <xiyou.wangcong@gmail.com>
> CC: Saeed Mahameed <saeedm@mellanox.com>
> CC: Or Gerlitz <ogerlitz@mellanox.com>
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 10 +---------
> drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 10 +---------
> drivers/net/ethernet/netronome/nfp/nfp_net_offload.c | 11 ++---------
> include/net/pkt_cls.h | 19 +++++++++++++++++++
> 4 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index d2f90ba2dbc4..7914a32a3036 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -1863,9 +1863,7 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
> {
> struct mlx5e_tc_table *tc = &priv->fs.tc;
> struct mlx5e_tc_flow *flow;
> - struct tc_action *a;
> struct mlx5_fc *counter;
> - LIST_HEAD(actions);
> u64 bytes;
> u64 packets;
> u64 lastuse;
> @@ -1884,13 +1882,7 @@ int mlx5e_stats_flower(struct mlx5e_priv *priv,
>
> mlx5_fc_query_cached(counter, &bytes, &packets, &lastuse);
>
> - preempt_disable();
> -
> - tcf_exts_to_list(f->exts, &actions);
> - list_for_each_entry(a, &actions, list)
> - tcf_action_stats_update(a, bytes, packets, lastuse);
> -
> - preempt_enable();
> + tcf_exts_stats_update(f->exts, bytes, packets, lastuse);
>
> return 0;
> }
> index f7762295b7b8..537d0a0ad4c4 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -157,6 +157,25 @@ static inline void tcf_exts_to_list(const struct tcf_exts *exts,
> #endif
> }
>
> +static inline void
> +tcf_exts_stats_update(const struct tcf_exts *exts,
> + u64 bytes, u64 packets, u64 lastuse)
> +{
> +#ifdef CONFIG_NET_CLS_ACT
> + int i;
> +
> + preempt_disable();
> +
> + for (i = 0; i < exts->nr_actions; i++) {
> + struct tc_action *a = exts->actions[i];
> +
> + tcf_action_stats_update(a, bytes, packets, lastuse);
> + }
> +
> + preempt_enable();
> +#endif
> +}
looks ok, for the mlx5 and pkt_cls.h parts:
Acked-by: Or Gerlitz <ogerlitz@mellanox.com>
^ permalink raw reply
* [for 4.12] net: qcom/emac: do not use hardware mdio automatic polling
From: Timur Tabi @ 2017-06-01 21:05 UTC (permalink / raw)
To: linux-arm-kernel, David S. Miller, netdev, Manoj Iyer,
zefir.kurtisi, andrew, matthias.may, f.fainelli
Cc: timur
Use software polling (PHY_POLL) to check for link state changes instead
of relying on the EMAC's hardware polling feature. Some PHY drivers
are unable to get a functioning link because the HW polling is not
robust enough.
The EMAC is able to poll the PHY on the MDIO bus looking for link state
changes (via the Link Status bit in the Status Register at address 0x1).
When the link state changes, the EMAC triggers an interrupt and tells the
driver what the new state is. The feature eliminates the need for
software to poll the MDIO bus.
Unfortunately, this feature is incompatible with phylib, because it
ignores everything that the PHY core and PHY drivers are trying to do.
In particular:
1. It assumes a compatible register set, so PHYs with different registers
may not work.
2. It doesn't allow for hardware errata that have work-arounds implemented
in the PHY driver.
3. It doesn't support multiple register pages. If the PHY core switches
the register set to another page, the EMAC won't know the page has
changed and will still attempt to read the same PHY register.
4. It only checks the copper side of the link, not the SGMII side. Some
PHY drivers (e.g. at803x) may also check the SGMII side, and
report the link as not ready during autonegotiation if the SGMII link
is still down. Phylib then waits for another interrupt to query
the PHY again, but the EMAC won't send another interrupt because it
thinks the link is up.
Cc: stable@vger.kernel.org # 4.11.x
Tested-by: Manoj Iyer <manoj.iyer@canonical.com>
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 2 +-
drivers/net/ethernet/qualcomm/emac/emac-phy.c | 75 ++-------------------------
drivers/net/ethernet/qualcomm/emac/emac.c | 22 +-------
3 files changed, 6 insertions(+), 93 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index e635605..1c0dbaa 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -932,7 +932,7 @@ int emac_mac_up(struct emac_adapter *adpt)
emac_mac_config(adpt);
emac_mac_rx_descs_refill(adpt, &adpt->rx_q);
- adpt->phydev->irq = PHY_IGNORE_INTERRUPT;
+ adpt->phydev->irq = PHY_POLL;
ret = phy_connect_direct(netdev, adpt->phydev, emac_adjust_link,
PHY_INTERFACE_MODE_SGMII);
if (ret) {
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index 441c1936..18461fc 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
@@ -13,15 +13,11 @@
/* Qualcomm Technologies, Inc. EMAC PHY Controller driver.
*/
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_net.h>
#include <linux/of_mdio.h>
#include <linux/phy.h>
#include <linux/iopoll.h>
#include <linux/acpi.h>
#include "emac.h"
-#include "emac-mac.h"
/* EMAC base register offsets */
#define EMAC_MDIO_CTRL 0x001414
@@ -52,62 +48,10 @@
#define MDIO_WAIT_TIMES 1000
-#define EMAC_LINK_SPEED_DEFAULT (\
- EMAC_LINK_SPEED_10_HALF |\
- EMAC_LINK_SPEED_10_FULL |\
- EMAC_LINK_SPEED_100_HALF |\
- EMAC_LINK_SPEED_100_FULL |\
- EMAC_LINK_SPEED_1GB_FULL)
-
-/**
- * emac_phy_mdio_autopoll_disable() - disable mdio autopoll
- * @adpt: the emac adapter
- *
- * The autopoll feature takes over the MDIO bus. In order for
- * the PHY driver to be able to talk to the PHY over the MDIO
- * bus, we need to temporarily disable the autopoll feature.
- */
-static int emac_phy_mdio_autopoll_disable(struct emac_adapter *adpt)
-{
- u32 val;
-
- /* disable autopoll */
- emac_reg_update32(adpt->base + EMAC_MDIO_CTRL, MDIO_AP_EN, 0);
-
- /* wait for any mdio polling to complete */
- if (!readl_poll_timeout(adpt->base + EMAC_MDIO_CTRL, val,
- !(val & MDIO_BUSY), 100, MDIO_WAIT_TIMES * 100))
- return 0;
-
- /* failed to disable; ensure it is enabled before returning */
- emac_reg_update32(adpt->base + EMAC_MDIO_CTRL, 0, MDIO_AP_EN);
-
- return -EBUSY;
-}
-
-/**
- * emac_phy_mdio_autopoll_disable() - disable mdio autopoll
- * @adpt: the emac adapter
- *
- * The EMAC has the ability to poll the external PHY on the MDIO
- * bus for link state changes. This eliminates the need for the
- * driver to poll the phy. If if the link state does change,
- * the EMAC issues an interrupt on behalf of the PHY.
- */
-static void emac_phy_mdio_autopoll_enable(struct emac_adapter *adpt)
-{
- emac_reg_update32(adpt->base + EMAC_MDIO_CTRL, 0, MDIO_AP_EN);
-}
-
static int emac_mdio_read(struct mii_bus *bus, int addr, int regnum)
{
struct emac_adapter *adpt = bus->priv;
u32 reg;
- int ret;
-
- ret = emac_phy_mdio_autopoll_disable(adpt);
- if (ret)
- return ret;
emac_reg_update32(adpt->base + EMAC_PHY_STS, PHY_ADDR_BMSK,
(addr << PHY_ADDR_SHFT));
@@ -122,24 +66,15 @@ static int emac_mdio_read(struct mii_bus *bus, int addr, int regnum)
if (readl_poll_timeout(adpt->base + EMAC_MDIO_CTRL, reg,
!(reg & (MDIO_START | MDIO_BUSY)),
100, MDIO_WAIT_TIMES * 100))
- ret = -EIO;
- else
- ret = (reg >> MDIO_DATA_SHFT) & MDIO_DATA_BMSK;
+ return -EIO;
- emac_phy_mdio_autopoll_enable(adpt);
-
- return ret;
+ return (reg >> MDIO_DATA_SHFT) & MDIO_DATA_BMSK;
}
static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
{
struct emac_adapter *adpt = bus->priv;
u32 reg;
- int ret;
-
- ret = emac_phy_mdio_autopoll_disable(adpt);
- if (ret)
- return ret;
emac_reg_update32(adpt->base + EMAC_PHY_STS, PHY_ADDR_BMSK,
(addr << PHY_ADDR_SHFT));
@@ -155,11 +90,9 @@ static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
if (readl_poll_timeout(adpt->base + EMAC_MDIO_CTRL, reg,
!(reg & (MDIO_START | MDIO_BUSY)), 100,
MDIO_WAIT_TIMES * 100))
- ret = -EIO;
+ return -EIO;
- emac_phy_mdio_autopoll_enable(adpt);
-
- return ret;
+ return 0;
}
/* Configure the MDIO bus and connect the external PHY */
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 28a8cdc..98a326f 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -50,19 +50,7 @@
#define DMAR_DLY_CNT_DEF 15
#define DMAW_DLY_CNT_DEF 4
-#define IMR_NORMAL_MASK (\
- ISR_ERROR |\
- ISR_GPHY_LINK |\
- ISR_TX_PKT |\
- GPHY_WAKEUP_INT)
-
-#define IMR_EXTENDED_MASK (\
- SW_MAN_INT |\
- ISR_OVER |\
- ISR_ERROR |\
- ISR_GPHY_LINK |\
- ISR_TX_PKT |\
- GPHY_WAKEUP_INT)
+#define IMR_NORMAL_MASK (ISR_ERROR | ISR_OVER | ISR_TX_PKT)
#define ISR_TX_PKT (\
TX_PKT_INT |\
@@ -70,10 +58,6 @@
TX_PKT_INT2 |\
TX_PKT_INT3)
-#define ISR_GPHY_LINK (\
- GPHY_LINK_UP_INT |\
- GPHY_LINK_DOWN_INT)
-
#define ISR_OVER (\
RFD0_UR_INT |\
RFD1_UR_INT |\
@@ -187,10 +171,6 @@ irqreturn_t emac_isr(int _irq, void *data)
if (status & ISR_OVER)
net_warn_ratelimited("warning: TX/RX overflow\n");
- /* link event */
- if (status & ISR_GPHY_LINK)
- phy_mac_interrupt(adpt->phydev, !!(status & GPHY_LINK_UP_INT));
-
exit:
/* enable the interrupt */
writel(irq->mask, adpt->base + EMAC_INT_MASK);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [for 4.12] net: qcom/emac: do not use hardware mdio automatic polling
From: Timur Tabi @ 2017-06-01 21:08 UTC (permalink / raw)
To: linux-arm-kernel, David S. Miller, Florian Fainelli, Manoj Iyer,
netdev, stable, zefir.kurtisi, andrew, matthias.may
Cc: timur
Use software polling (PHY_POLL) to check for link state changes instead
of relying on the EMAC's hardware polling feature. Some PHY drivers
are unable to get a functioning link because the HW polling is not
robust enough.
The EMAC is able to poll the PHY on the MDIO bus looking for link state
changes (via the Link Status bit in the Status Register at address 0x1).
When the link state changes, the EMAC triggers an interrupt and tells the
driver what the new state is. The feature eliminates the need for
software to poll the MDIO bus.
Unfortunately, this feature is incompatible with phylib, because it
ignores everything that the PHY core and PHY drivers are trying to do.
In particular:
1. It assumes a compatible register set, so PHYs with different registers
may not work.
2. It doesn't allow for hardware errata that have work-arounds implemented
in the PHY driver.
3. It doesn't support multiple register pages. If the PHY core switches
the register set to another page, the EMAC won't know the page has
changed and will still attempt to read the same PHY register.
4. It only checks the copper side of the link, not the SGMII side. Some
PHY drivers (e.g. at803x) may also check the SGMII side, and
report the link as not ready during autonegotiation if the SGMII link
is still down. Phylib then waits for another interrupt to query
the PHY again, but the EMAC won't send another interrupt because it
thinks the link is up.
Cc: stable@vger.kernel.org # 4.11.x
Tested-by: Manoj Iyer <manoj.iyer@canonical.com>
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 2 +-
drivers/net/ethernet/qualcomm/emac/emac-phy.c | 75 ++-------------------------
drivers/net/ethernet/qualcomm/emac/emac.c | 22 +-------
3 files changed, 6 insertions(+), 93 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index e635605..1c0dbaa 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -932,7 +932,7 @@ int emac_mac_up(struct emac_adapter *adpt)
emac_mac_config(adpt);
emac_mac_rx_descs_refill(adpt, &adpt->rx_q);
- adpt->phydev->irq = PHY_IGNORE_INTERRUPT;
+ adpt->phydev->irq = PHY_POLL;
ret = phy_connect_direct(netdev, adpt->phydev, emac_adjust_link,
PHY_INTERFACE_MODE_SGMII);
if (ret) {
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index 441c1936..18461fc 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
@@ -13,15 +13,11 @@
/* Qualcomm Technologies, Inc. EMAC PHY Controller driver.
*/
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_net.h>
#include <linux/of_mdio.h>
#include <linux/phy.h>
#include <linux/iopoll.h>
#include <linux/acpi.h>
#include "emac.h"
-#include "emac-mac.h"
/* EMAC base register offsets */
#define EMAC_MDIO_CTRL 0x001414
@@ -52,62 +48,10 @@
#define MDIO_WAIT_TIMES 1000
-#define EMAC_LINK_SPEED_DEFAULT (\
- EMAC_LINK_SPEED_10_HALF |\
- EMAC_LINK_SPEED_10_FULL |\
- EMAC_LINK_SPEED_100_HALF |\
- EMAC_LINK_SPEED_100_FULL |\
- EMAC_LINK_SPEED_1GB_FULL)
-
-/**
- * emac_phy_mdio_autopoll_disable() - disable mdio autopoll
- * @adpt: the emac adapter
- *
- * The autopoll feature takes over the MDIO bus. In order for
- * the PHY driver to be able to talk to the PHY over the MDIO
- * bus, we need to temporarily disable the autopoll feature.
- */
-static int emac_phy_mdio_autopoll_disable(struct emac_adapter *adpt)
-{
- u32 val;
-
- /* disable autopoll */
- emac_reg_update32(adpt->base + EMAC_MDIO_CTRL, MDIO_AP_EN, 0);
-
- /* wait for any mdio polling to complete */
- if (!readl_poll_timeout(adpt->base + EMAC_MDIO_CTRL, val,
- !(val & MDIO_BUSY), 100, MDIO_WAIT_TIMES * 100))
- return 0;
-
- /* failed to disable; ensure it is enabled before returning */
- emac_reg_update32(adpt->base + EMAC_MDIO_CTRL, 0, MDIO_AP_EN);
-
- return -EBUSY;
-}
-
-/**
- * emac_phy_mdio_autopoll_disable() - disable mdio autopoll
- * @adpt: the emac adapter
- *
- * The EMAC has the ability to poll the external PHY on the MDIO
- * bus for link state changes. This eliminates the need for the
- * driver to poll the phy. If if the link state does change,
- * the EMAC issues an interrupt on behalf of the PHY.
- */
-static void emac_phy_mdio_autopoll_enable(struct emac_adapter *adpt)
-{
- emac_reg_update32(adpt->base + EMAC_MDIO_CTRL, 0, MDIO_AP_EN);
-}
-
static int emac_mdio_read(struct mii_bus *bus, int addr, int regnum)
{
struct emac_adapter *adpt = bus->priv;
u32 reg;
- int ret;
-
- ret = emac_phy_mdio_autopoll_disable(adpt);
- if (ret)
- return ret;
emac_reg_update32(adpt->base + EMAC_PHY_STS, PHY_ADDR_BMSK,
(addr << PHY_ADDR_SHFT));
@@ -122,24 +66,15 @@ static int emac_mdio_read(struct mii_bus *bus, int addr, int regnum)
if (readl_poll_timeout(adpt->base + EMAC_MDIO_CTRL, reg,
!(reg & (MDIO_START | MDIO_BUSY)),
100, MDIO_WAIT_TIMES * 100))
- ret = -EIO;
- else
- ret = (reg >> MDIO_DATA_SHFT) & MDIO_DATA_BMSK;
+ return -EIO;
- emac_phy_mdio_autopoll_enable(adpt);
-
- return ret;
+ return (reg >> MDIO_DATA_SHFT) & MDIO_DATA_BMSK;
}
static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
{
struct emac_adapter *adpt = bus->priv;
u32 reg;
- int ret;
-
- ret = emac_phy_mdio_autopoll_disable(adpt);
- if (ret)
- return ret;
emac_reg_update32(adpt->base + EMAC_PHY_STS, PHY_ADDR_BMSK,
(addr << PHY_ADDR_SHFT));
@@ -155,11 +90,9 @@ static int emac_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
if (readl_poll_timeout(adpt->base + EMAC_MDIO_CTRL, reg,
!(reg & (MDIO_START | MDIO_BUSY)), 100,
MDIO_WAIT_TIMES * 100))
- ret = -EIO;
+ return -EIO;
- emac_phy_mdio_autopoll_enable(adpt);
-
- return ret;
+ return 0;
}
/* Configure the MDIO bus and connect the external PHY */
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 28a8cdc..98a326f 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -50,19 +50,7 @@
#define DMAR_DLY_CNT_DEF 15
#define DMAW_DLY_CNT_DEF 4
-#define IMR_NORMAL_MASK (\
- ISR_ERROR |\
- ISR_GPHY_LINK |\
- ISR_TX_PKT |\
- GPHY_WAKEUP_INT)
-
-#define IMR_EXTENDED_MASK (\
- SW_MAN_INT |\
- ISR_OVER |\
- ISR_ERROR |\
- ISR_GPHY_LINK |\
- ISR_TX_PKT |\
- GPHY_WAKEUP_INT)
+#define IMR_NORMAL_MASK (ISR_ERROR | ISR_OVER | ISR_TX_PKT)
#define ISR_TX_PKT (\
TX_PKT_INT |\
@@ -70,10 +58,6 @@
TX_PKT_INT2 |\
TX_PKT_INT3)
-#define ISR_GPHY_LINK (\
- GPHY_LINK_UP_INT |\
- GPHY_LINK_DOWN_INT)
-
#define ISR_OVER (\
RFD0_UR_INT |\
RFD1_UR_INT |\
@@ -187,10 +171,6 @@ irqreturn_t emac_isr(int _irq, void *data)
if (status & ISR_OVER)
net_warn_ratelimited("warning: TX/RX overflow\n");
- /* link event */
- if (status & ISR_GPHY_LINK)
- phy_mac_interrupt(adpt->phydev, !!(status & GPHY_LINK_UP_INT));
-
exit:
/* enable the interrupt */
writel(irq->mask, adpt->base + EMAC_INT_MASK);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH net-next] tcp: remove unnecessary skb_reset_tail_pointer()
From: Eric Dumazet @ 2017-06-01 21:18 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
__pskb_trim_head() does not need to reset skb tail pointer.
Also change the comments, __pskb_pull_head() does not exist.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_output.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 478f75baee31d28b4e3122f7635cd1addf20cb98..e3aab1c1cf78d7b5696fc1438fe54f8ce448c09e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1328,9 +1328,8 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len,
return 0;
}
-/* This is similar to __pskb_pull_head() (it will go to core/skbuff.c
- * eventually). The difference is that pulled data not copied, but
- * immediately discarded.
+/* This is similar to __pskb_pull_tail(). The difference is that pulled
+ * data is not copied, but immediately discarded.
*/
static int __pskb_trim_head(struct sk_buff *skb, int len)
{
@@ -1365,7 +1364,6 @@ static int __pskb_trim_head(struct sk_buff *skb, int len)
}
shinfo->nr_frags = k;
- skb_reset_tail_pointer(skb);
skb->data_len -= len;
skb->len = skb->data_len;
return len;
^ permalink raw reply related
* Re: [PATCH net-next v3 0/5] net: dsa: tagger simplification
From: David Miller @ 2017-06-01 21:36 UTC (permalink / raw)
To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew
In-Reply-To: <20170601200715.8438-1-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Thu, 1 Jun 2017 16:07:10 -0400
> This series clarifies the hot path, removes the labels in tagging
> implementations, and frees the original SKB in the xmit caller.
>
> Changes in v3:
> - drop removal of usused rcv arguments because they will be used later
> - include the new ksz tagging implementation
> - add reviewers' tags
>
> Changes in v2:
> - do not remove tagger function copies
> - document hot path requirements
> - make netdev_uses_dsa simpler
> - add reviewers' tags
Series applied, thanks Vivien.
^ permalink raw reply
* [net-next 00/14][pull request] 40GbE Intel Wired LAN Driver Updates 2017-06-01
From: Jeff Kirsher @ 2017-06-01 21:38 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains updates to i40e, i40evf and the "new" AVF virtchnl.
This is the introduction of the Intel(R) Ethernet Adaptive Virtual
Function driver code and device ID, as presented at the NetDEV 1.2
conference in 2016.
http://netdevconf.org/1.2/session.html?anjali-singhai
The idea is to convert the interface between the i40evf driver
and the parent i40e PF driver to be generic, as the i40evf driver
should in the future be able to run on top of other Intel PF
drivers, and negotiate any features beyond a "base expected" set.
The following are changes since commit 0266f79778de0afadd070941aae493c28529d974:
mlxsw: spectrum: Add bridge dependency for spectrum
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE
Jesse Brandeburg (11):
i40evf: drop i40e_type.h include
i40e/i40evf: create and use new unified header file
i40e: use new unified virtchnl header file
virtchnl: rename i40e to generic virtchnl
virtchnl: move some code to core driver
virtchnl: convert to new macros
i40e/virtchnl: refactor code for validate checks
i40evf/virtchnl: whitespace cleanups
virtchnl: finish conversion to virtchnl interface
i40e/virtchnl: move function to virtchnl
i40evf: update i40evf.txt with new content
Preethi Banala (1):
i40evf: Add support for Adaptive Virtual Function
Sridhar Samudrala (2):
virtchnl: Add pad fields to a couple of structures
virtchnl: Add compile time static asserts to validate structure sizes
Documentation/networking/i40evf.txt | 23 +-
MAINTAINERS | 1 +
drivers/net/ethernet/intel/Kconfig | 10 +-
drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_common.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_prototype.h | 6 +-
drivers/net/ethernet/intel/i40e/i40e_virtchnl.h | 449 -------------
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 474 +++++---------
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h | 9 +-
drivers/net/ethernet/intel/i40evf/i40e_common.c | 15 +-
drivers/net/ethernet/intel/i40evf/i40e_devids.h | 1 +
drivers/net/ethernet/intel/i40evf/i40e_prototype.h | 6 +-
drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h | 448 -------------
drivers/net/ethernet/intel/i40evf/i40evf.h | 26 +-
drivers/net/ethernet/intel/i40evf/i40evf_client.c | 18 +-
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 66 +-
.../net/ethernet/intel/i40evf/i40evf_virtchnl.c | 285 +++++----
include/linux/avf/virtchnl.h | 701 +++++++++++++++++++++
19 files changed, 1109 insertions(+), 1435 deletions(-)
delete mode 100644 drivers/net/ethernet/intel/i40e/i40e_virtchnl.h
delete mode 100644 drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
create mode 100644 include/linux/avf/virtchnl.h
--
2.12.2
^ permalink raw reply
* [net-next 02/14] i40e/i40evf: create and use new unified header file
From: Jeff Kirsher @ 2017-06-01 21:38 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20170601213840.36286-1-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
This moves a header for i40evf to include/linux/avf/virtchnl.h.
The directory name AVF is an acronym for the Intel(R) Adaptive
Virtual Function.
This first step creates the new file, which is a rename of
drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h to
include/linux/avf/virtchnl.h, and should show up in git
as a rename when using git log --follow.
To keep things building after the move, the changes to the i40evf
driver are made to point to the new include file location.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
MAINTAINERS | 1 +
drivers/net/ethernet/intel/i40evf/i40e_common.c | 2 +-
drivers/net/ethernet/intel/i40evf/i40e_prototype.h | 2 +-
drivers/net/ethernet/intel/i40evf/i40evf.h | 2 +-
.../intel/i40evf/i40e_virtchnl.h => include/linux/avf/virtchnl.h | 0
5 files changed, 4 insertions(+), 3 deletions(-)
rename drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h => include/linux/avf/virtchnl.h (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0fcb5e751ca7..6b7625ff9875 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6738,6 +6738,7 @@ F: Documentation/networking/i40e.txt
F: Documentation/networking/i40evf.txt
F: drivers/net/ethernet/intel/
F: drivers/net/ethernet/intel/*/
+F: include/linux/avf/virtchnl.h
INTEL RDMA RNIC DRIVER
M: Faisal Latif <faisal.latif@intel.com>
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_common.c b/drivers/net/ethernet/intel/i40evf/i40e_common.c
index 6729624fda5b..1db028ac96f4 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_common.c
@@ -27,7 +27,7 @@
#include "i40e_type.h"
#include "i40e_adminq.h"
#include "i40e_prototype.h"
-#include "i40e_virtchnl.h"
+#include <linux/avf/virtchnl.h>
/**
* i40e_set_mac_type - Sets MAC type
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_prototype.h b/drivers/net/ethernet/intel/i40evf/i40e_prototype.h
index 741223d5d809..227905b23690 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_prototype.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_prototype.h
@@ -29,7 +29,7 @@
#include "i40e_type.h"
#include "i40e_alloc.h"
-#include "i40e_virtchnl.h"
+#include <linux/avf/virtchnl.h>
/* Prototypes for shared code functions that are not in
* the standard function pointer structures. These are
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf.h b/drivers/net/ethernet/intel/i40evf/i40evf.h
index b8ada6d8d890..75d314b1a9bb 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf.h
+++ b/drivers/net/ethernet/intel/i40evf/i40evf.h
@@ -43,7 +43,7 @@
#include <net/udp.h>
#include "i40e_type.h"
-#include "i40e_virtchnl.h"
+#include <linux/avf/virtchnl.h>
#include "i40e_txrx.h"
#define DEFAULT_DEBUG_LEVEL_SHIFT 3
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h b/include/linux/avf/virtchnl.h
similarity index 100%
rename from drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
rename to include/linux/avf/virtchnl.h
--
2.12.2
^ permalink raw reply related
* [net-next 01/14] i40evf: drop i40e_type.h include
From: Jeff Kirsher @ 2017-06-01 21:38 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20170601213840.36286-1-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
This drops the i40e_type.h include in anticipation of the next
patch which moves this file to a location where type.h doesn't
exist, and all the places this file is included already include
i40e_type.h before this file.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h b/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
index b0b8de5d6f57..7d6da3ac24f4 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_virtchnl.h
@@ -27,8 +27,6 @@
#ifndef _I40E_VIRTCHNL_H_
#define _I40E_VIRTCHNL_H_
-#include "i40e_type.h"
-
/* Description:
* This header file describes the VF-PF communication protocol used
* by the various i40e drivers.
--
2.12.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox