* Re: [PATCH] net: mlxsw: avoid unused variable warnings
From: David Miller @ 2016-03-23 18:38 UTC (permalink / raw)
To: andrew; +Cc: arnd, jiri, idosch, eladr, netdev, linux-kernel
In-Reply-To: <20160323165111.GJ5250@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 23 Mar 2016 17:51:11 +0100
> On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
>> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
>> not set, which can lead to unused variable warnings:
>>
>> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
>> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
>> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
>> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
>>
>> This changes the mlxsw driver to remove the local variables we get
>> warnings for and instead pass the device directly into the API.
>
> Hi Arnd
>
> Would it not be better to fix the macro?
>
> I think the issue is that dev_dbg_ratelimited calls no_printk(),
> without making use of dev. So how about:
>
> #define dev_dbg_ratelimited(dev, fmt, ...) \
> ({ \
> if (0) \
> dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> })
>
> This follows the pattern for other macros for when DEBUG is not defined.
Yeah, this is probably a better way to fix this problem.
^ permalink raw reply
* Re: [PATCH net] ppp: take reference on channels netns
From: David Miller @ 2016-03-23 18:37 UTC (permalink / raw)
To: g.nault; +Cc: netdev, gorcunov, sploving1, paulus
In-Reply-To: <4dbdfa8e264d5a635865c6e843c122daf9be4f8d.1458747209.git.g.nault@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Wed, 23 Mar 2016 16:38:55 +0100
> Let channels hold a reference on their network namespace.
> Some channel types, like ppp_async and ppp_synctty, can have their
> userspace controller running in a different namespace. Therefore they
> can't rely on them to preclude their netns from being removed from
> under them.
...
> Fixes: 273ec51dd7ce ("net: ppp_generic - introduce net-namespace functionality v2")
> Reported-by: Baozeng Ding <sploving1@gmail.com>
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH] vlan: propagate gso_min_segs
From: Eric Dumazet @ 2016-03-23 18:34 UTC (permalink / raw)
To: David Miller; +Cc: yanhaishuang, kaber, nicolas.dichtel, netdev, linux-kernel
In-Reply-To: <20160323.142556.2023977823696366302.davem@davemloft.net>
On Wed, 2016-03-23 at 14:25 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 22 Mar 2016 19:33:52 -0700
>
> > On Wed, 2016-03-23 at 09:35 +0800, Haishuang Yan wrote:
> >> vlan drivers lack proper propagation of gso_min_segs from lower device.
> >>
> >> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
> >> ---
> >
> > The plan was to get rid of gso_min_segs, as nothing uses it.
> >
> > Otherwise I would have included this in my recent patches...
> >
> > For such a rare 'issue' , we believe ndo_features_check() is better,
> > because it does not slow down the fast path.
>
> I'm not aplpying this.
I am adding to my TODO list a revert, when net-next opens again.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: Fix typos and whitespace.
From: David Miller @ 2016-03-23 18:27 UTC (permalink / raw)
To: bhelgaas; +Cc: netdev, linux-kernel
In-Reply-To: <20160323134530.18742.35082.stgit@bhelgaas-glaptop2.roam.corp.google.com>
From: Bjorn Helgaas <bhelgaas@google.com>
Date: Wed, 23 Mar 2016 08:45:30 -0500
> Fix typos. Capitalize CPU, NAPI, RCU consistently. Align structure
> indentation. No functional change intended; only comment and whitespace
> changes.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Does not apply to the current 'net' tree, please respin.
^ permalink raw reply
* Re: [PATCH] vlan: propagate gso_min_segs
From: David Miller @ 2016-03-23 18:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: yanhaishuang, kaber, nicolas.dichtel, netdev, linux-kernel
In-Reply-To: <1458700432.10868.32.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 22 Mar 2016 19:33:52 -0700
> On Wed, 2016-03-23 at 09:35 +0800, Haishuang Yan wrote:
>> vlan drivers lack proper propagation of gso_min_segs from lower device.
>>
>> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
>> ---
>
> The plan was to get rid of gso_min_segs, as nothing uses it.
>
> Otherwise I would have included this in my recent patches...
>
> For such a rare 'issue' , we believe ndo_features_check() is better,
> because it does not slow down the fast path.
I'm not aplpying this.
^ permalink raw reply
* Re: [PATCH] gre: fix return value of gre_rcv
From: David Miller @ 2016-03-23 18:25 UTC (permalink / raw)
To: yanhaishuang; +Cc: kuznet, jmorris, kaber, netdev, linux-kernel
In-Reply-To: <1458696883-15796-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
From: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Date: Wed, 23 Mar 2016 09:34:43 +0800
> Dropped skb's should be documented by an appropriate return value.
> Use the correct NET_RX_DROP and NET_RX_SUCCESS values for that reason.
>
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
IP protocol handlers do not use NET_RX_* values like this.
They either return 0, to finish processing, or a negative value which
indicates the negated protocol number to use to demux in the protocol
switch again.
Many of your patches seem very straightforward, but usually upon
further inspection severe fundamental issues with your changes are
found because you don't know what the proper semantics are for the
pieces of infrastructure you are touching.
Therefore, reviewing your seemingly trivial changes is a large
inventment of time and energy.
Please put more care into your changes, or else people will simply
stop reviewing your work and instead put their effort into submissions
which are more properly researched and easier to verify.
Thanks.
^ permalink raw reply
* Re: [net PATCH] net: Reset encap_level to avoid resetting features on inner IP headers
From: David Miller @ 2016-03-23 18:19 UTC (permalink / raw)
To: aduyck; +Cc: tom, kernel-team, alexander.duyck, netdev
In-Reply-To: <20160322231732.13054.71337.stgit@localhost.localdomain>
From: Alexander Duyck <aduyck@mirantis.com>
Date: Tue, 22 Mar 2016 16:18:07 -0700
> This patch corrects an oversight in which we were allowing the encap_level
> value to pass from the outer headers to the inner headers. As a result we
> were incorrectly identifying UDP or GRE tunnels as also making use of ipip
> or sit when the second header actually represented a tunnel encapsulated in
> either a UDP or GRE tunnel which already had the features masked.
>
> Fixes: 76443456227097179c1482 ("net: Move GSO csum into SKB_GSO_CB")
> Reported-by: Tom Herbert <tom@herbertland.com>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Applied, thanks.
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-23 18:19 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, Edward Cree, Linux Kernel Network Developers,
David S. Miller
In-Reply-To: <CALx6S35wK2_eR1e5OTHnQTkQRg763unS0sCMQU+Jk6e27F26Vw@mail.gmail.com>
On Wed, Mar 23, 2016 at 10:09 AM, Tom Herbert <tom@herbertland.com> wrote:
> On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This patch adds support for something I am referring to as GSO partial.
>> The basic idea is that we can support a broader range of devices for
>> segmentation if we use fixed outer headers and have the hardware only
>> really deal with segmenting the inner header. The idea behind the naming
>> is due to the fact that everything before csum_start will be fixed headers,
>> and everything after will be the region that is handled by hardware.
>>
> Personally, I don't like the name ;-) This technique should be
> "generic" to handle almost all GSO except those case where headers are
> not fixed which we should be able to avoid as a design point in any
> new encapsulations. Besides, what if someday we perform GSO on
> something where csum_start is not set?
Well the "partial" component of it refers to the fact that we have to
do a certain amount of this in software before we offload the work to
hardware. So it isn't a full segmentation offload, it is a partial
one.
The csum_start is a bit of a red herring in terms of the naming. I
was using csum_start as that works for TCP offloads since it will
always point to the inner-most transport header. It was basically
just a convenient optimization for the drivers and makes it so that we
can essentially use just one code path in the case of ixgbe since
network header and checksum_start will always represent the two
headers we need to update for TSO regardless of it being partial or
complete offload.
> Can you add some description about strategy for dealing with ip_id?
Yeah. I still need to add more documentation. I just didn't want to
get into details on it until we have finalized things a bit more. I'm
still wondering if we should follow the pattern of ipip tunnels in
terms of setting the DF bit if the inner header has the DF bit set.
If we end up needing to add code to do that then it makes it so that
the ip_id value can be fixed for both inner and outer which makes the
segmentation much simpler since the only header that would ever really
need to be updated would be the transport header in order to get the
checksum correct.
- Alex
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Alexander Duyck @ 2016-03-23 18:06 UTC (permalink / raw)
To: Edward Cree, Or Gerlitz
Cc: Alexander Duyck, Netdev, David Miller, Tom Herbert
In-Reply-To: <56F2C3F8.8070308@solarflare.com>
On Wed, Mar 23, 2016 at 9:27 AM, Edward Cree <ecree@solarflare.com> wrote:
> On 22/03/16 21:38, Alexander Duyck wrote:
>> On Tue, Mar 22, 2016 at 12:40 PM, Edward Cree <ecree@solarflare.com> wrote:
>>> But won't the tunnel dev have the feature flag for GSO_PARTIAL depending
>>> on what the underlying dev advertises? (Or, at least, could we make it
>>> bethatway?)
>> This stuff doesn't work. That is why tunnels now advertise all
>> available features that can be offloaded via software. Basically if
>> we can advertise a feature we do, and then we sort things out in
>> software if we cannot actually do it in hardware.
> Fair enough; then go withthe other approach:
>>> Alternatively, have per-protocol GSO callbacks to do the fixup in the
>>> opposite direction to what you have now - in the long term we hope that
>>> hardware supporting GSO partial will become the common case, so that
>>> should be the fast path without bouncing backwards through GSO callbacks.
>>> Then, if you find out at GSO time that the hardware wants to do old-style
>>> TSO, you call those callbacks so as to give it a superframe with the long
>>> lengths filled in everywhere.
>> I thought about doing that but decided it was much simpler to simply
>> update all headers. For now I want to keep this as simple as possible
>> while we get the first few drivers on board. If we later want to
>> optimize and add complexity we can go that route, but for now this
>> change is more than fast enough as it allows me to push an i40e at
>> 20Gb/s while sending frames with outer checksums enabled.
> My belief is that my way is (in the long run) simpler: ultimately it gets
> rid of per-protocol GSO callbacks entirely. Every header gets written
> correctly* when the packet initially traverses the stack, and then at
> transmit time you either hand that off to TSO, or do the equivalent thing
> in software: segment at the TCP layer while treating everything above it
> as an opaque pseudo-L2 header.
I'm pretty sure that isn't a safe approach to take. With GSO we are
holding off until we are about to hand the packets off to the device
before we segment it. If we do it earlier it will lead to more issues
as you could have the packet route off to somewhere you were not
expecting and having it already "soft segmented" could lead to issues
where the packet would no longer be coherent.
> *That is, correctly for a single segment, rather than correctly for the
> superframe.
Yes, but what about the cases where a packets gets switched from Tx
back to Rx? How would you expect the stack to handle that then?
>>> Yes; it's a clever idea. Only trouble is that we really want theinner IP
>>> header rather than the inner TCP header, so that we can (if we want to)
>>> increment the inner IP IDs. Of course, if we Officially Don't Care about
>>> inner IP IDs that's not a problem.
>> The inner IP IDs are the ones that are guaranteed to be the ones we
>> can leave fixed since TCP will require that the DF bit be set.
> I was worrying about the SLHC stuff, I thought the inner ones were precisely
> the ones where that was a problem. If we really don't care about it, then
> we do just need the inner TCP header, and we can stick with using your
> csum_start == gso_start trick :)
>> The
>> current VXLAN implementation does not set DF for the outer headers.
>> So really we don't have too many options right now if we are wanting
>> to support tunnels.
> I was predicating all this on the assumption that tunnels would be changed
> to set DF in their outer frames; I thought I saw a patch recently to do
> that, but maybe I was mistaken. In any case I think that's the right thing
> to do, and it's a necessary prerequisite for truly tunnel-agnostic TSO.
>>> Iwonder if we could just always fill in inner_network_headereven if we're
>>> not doing encapsulation. Or does it end up pointing to a 'middle' header
>>> in the case of nested encap?
>> Right now neseted encap is not supported because tunnels don't
>> advertise hw_encap_features.
> You mean not supported by offloads, right? We can still _create_ a nested
> tunnel device, it just won't use hardware offloads. And in the long run,
> if we can make tunnel-agnostic TSO as I'm proposing, we should be able to
> support it for nested tunnels too (the giant L2 header just gets more giant).
Right. Basically what will currently happen is that if you were to
encapsulate an ipip tunnel inside of a VXLAN for instance the GSO
would kick in before the VXLAN tunnel has even added the outer headers
because the VXLAN netdev does not advertise NETIF_F_GSO_IPIP. That is
the kind of thing we want to have happen though anyway since a
physical device wouldn't know how to deal with such a scenario anyway.
>>> Why not use the single-segment length in the pseudo-header, then the
>>> outer L4 checksum is already the right thing? (And if yourhardware
>>> can't be told to leave the outer L4 checksum alone, then it's not worth
>>> the trouble of trying to support GSO partial for it, since it clearly
>>> wants to do old-style "NIC knows best" TSO.)
>> The problem then becomes that I needs special case code. One for
>> standard TSO and one for this special case. If I leave it as is I can
>> use the same code to cancel out the length in the TCP pseudo-header
>> checksum for either case.
> I don't think that's true. Look at it this way: there are two choices.
> 1) The normal path builds things for standard TSO, and we have code to
> fix it up for this new-style TSO.
> 2) The normal path builds things for new-style TSO, and we have code to
> fix it up for standard TSO.
> Now the fix-up code should be of equal complexity in either case, because
> the two cases are inverses of each other. So we should choose whichever
> approach makes the normal path simpler. I think having TCP 'lie' to all
> the outer layers about the length of the packet is simpler, because then
> they don't even have to know that GSO is happening. They just append a
> header appropriate for an MSS-sized packet, blissfully unaware that the
> payload data carries on for longer than that.
> Moreover, the fixup in (2) can sometimes be avoided...
> I don't know how your hardware does encapsulated TSO, but what ours does
> (in the old "I'm-a-smart-NIC-I-know-best" world) is that it computes both
> checksums itself from scratch. So it doesn't care what it was given as
> the pre-seeded value, because it's reconstructing the pseudo-header and
> zeroing the checksum field to begin with.
> I would have expected this to be the common behaviour for "smart" NICs
> doing encap TSO, in which case (2) is a clear winner. And, of course,
> once "less is more" hardware becomes common, we will want (2) anyway.
The argument for here is a matter of complexity. I really don't think
we gain much by recomputing the pseudo-header with the segmented
length versus the entire packet length. In the case of gso it is more
complicated to figure out the length if we are having to take gso_size
and add the size for a TCP header rather than just subtracting the
inner transport offset from skb->len.
>> The general idea to my approach is to treat the the UDP or GRE header
>> to the inner IP header as an IP header extension type value. That
>> will allow us to trick most hardware into being able to accept it.
> I think an L2 header extension is a better semantic match for what's
> happening (the tunnel is an L2 device and we're sending L3 packets over
> it). But why does it matter? Are you giving the hardware the L2 and
> L3 headers in separate DMA descriptors or something? The way I see it,
> all hardware needs to be told is "where to start TSO from", and how it
> thinks of the stuff before that doesn't matter, because it's not
> supposed to touch it anyway.
The problem is setups like VFs where they want to know where the
network header starts so they know where to insert a VLAN tag. Most
hardware supports IP options or IPv6 extension headers. What I am
doing is exploiting that in the case of the Intel hardware by telling
it the IP header is the outer IP header and the transport header is
the inner transport header. Then all I have to deal with is the fact
that hardware will try to compute the entire IPv4 header checksum over
the range so I cancel that out by seeding the IP checksum with the
lco_csum added to the inner pseudo-header checksum.
>>> Again, the idea is that we optimise for GSO partial by making it a plain
>>> header copy everywhere, and put all the 'fix things up' on the _other_
>>> path.
>> That is what I went for. The only part that isn't a plain header copy
>> is the TCP pseudo-header checksum.
> Right, but that means someone, somewhere, has to fix up the outer UDP
> checksum to account for that (because the TCP phdr sum leaks out of LCO).
> I realise that's a per-superframe job, rather than a per-segment job, but
> it still means that code at GSO time (when we decide whether to do GSO or
> GSO-partial) has to know enough about the tunnel headers to find the outer
> checksum and fix it up.
The fix-up already happens in the GSO code path I have coded up. The
outer checksum already takes it into account.
If you look at my patches you should see a block in both the UDP
tunnel and GRE tunnel code that handles the "skb_is_gso()" case. That
is where we deal with the checksum adjustment. In the case of UDP
tunnels it actually requires very little work as the only field that
actually has to be updated is uh->len since the outer pseduo-header
checksum effectively cancels out the length for the inner anyway. In
the case of GRE it took a bit more as I had to seed the checksum with
the delta of the length in order to correctly account for the change.
> And it's a question of who pays that cost, the old TSO or the new one; see
> above for why I think the old TSO should pay it.
Generally I don't agree with slowing down existing paths in order to
improve the performance for new ones. One of my goals with this was
to keep the code minimally intrusive. I would rather not have to
rewrite all driver TSO paths in order to support a new segmentation
approach.
>> The only issue I see is the expectation that the outer headers go
>> untouched is problematic. The outer headers are where things like
>> fragmentation will occur in transit.
> Like I say, I'm assuming we'll start setting DF on outer frames.
> Besides, it doesn't matter what happens "in transit" - as long as the
> outer headers aren't touched by the transmitting NIC, the network can
> do what it likes to them afterwards, without it breaking our TSO code.
The IP ID bit is going to be something where I don't want to break
things. One of the things I have seen is there ends up being a number
of occasions where VXLAN gets fragmented due to incorrectly configured
MTU. I would rather not have it get completely screwed up when this
occurs. A performance hit for fragmenting is one thing. Having
people start complaining because previously working tunnels suddenly
stop functioning is another. The fact is the whole point of VXLAN is
to support sending the tunneled frames between data centers. With the
DF bit set on a tunnel header we end up making it so that frames get
dropped instead of fragmented which would be a change of behavior.
>> In addition I suspect a number
>> of devices other than the Intel NICs probably use the network header
>> to determine where to insert L2 tags such as VLAN.
> Ah, VLAN is interesting, because there's two things you might want:
> VLAN inside the tunnel, or outside of it. Presumably if you're having
> the NIC insert the VLAN, you want it outside (e.g. you're doing SR-IOV
> and you're putting the VF on a VLAN).
> But then it doesn't make sense to work backwards from the network
> header, because that'll get confused by traffic that's already VLAN
> tagged - because again, you want to insert the new VLAN as the outer
> VLAN. You need to find the Ethertype, as well; it just seems like
> working backwards from L3 is crazy. Particularly when working forwards
> from L2 is so easy - you know your L2 header begins at the start of the
> packet, you (hopefully) know it's Ethernet, so you know where the VLAN
> tags and Ethertype go. (Are you /sure/ Intel works backwards from the
> L3 header? I'm pretty confident we don't.)
Yes, I am very confident of that. For Intel hardware the outer VLAN
tag would be the one inserted via software, the inner VLAN tag is the
one inserted via hardware.
> Then of course this works fine with the 'giant L2 header', because the
> NIC just inserts the VLAN as normal in the outer Ethernet header and
> shunts the remaining headers along to make room for it. In fact, in
> our NICs I think that's done by an entirely separate bit of hardware
> that doesn't even have to know that TSO was considering much more of
> the packet to be "L2 header".
Well the Intel hardware doesn't work that way. It would be nice if it
did because then you could actually support Q-in-Q on the VFs without
much issue, but due to the limiting nature of a 64b transmit
descriptor they opted to insert the VLAN tags at the start of the
network header.
> _However_, if we don't need to update the IP IDs, then we can just take
> the offset of the inner L4 header, and it doesn't make any difference
> whether you choose to think of the stuff before that as L2 + giant L3
> or as giant L2 + normal L3, because it's not part of the OS->NIC
> interface (which is just "L4 starts here"). If your NIC needs to be
> told where the outer L3 starts as well, then, I guess that's just a
> wart you need in your drivers. You have skb->network_header, so that
> shouldn't be difficult - that will always point to the outer one.
Right. That is kind of what I was going for with this setup. The
only issue is that the VXLAN tunnels not setting the DF bit kind of
get in the way of the giant L3 approach.
Dealing with the outer header needing the DF bit is something I have
left unaddressed at this point. The question would be what is the
correct approach to take for all this. I know RFC2003 for IPv4 in
IPv4 says you must set the DF bit if the inner header has the DF bit
set. I'm just wondering if we can apply the same type of logic to GRE
and UDP tunnels.
>> Like I have said with the current solution I could probably update
>> igb, igbvf, fm10k, ixgbe, ixgbevf, i40e, and i40evf to support this.
>> That covers pretty much the entire Intel series of drivers in terms of
>> enterprise. The question is what do I need to enable to support other
>> drivers. It doesn't seem like there would be too much but the bit I
>> would need to know is what the expectations are when computing outer
>> IPv4 checksums.
> Like I say, the plan we had for Solarflare NICs before "less is more"
> happened was that the device would reconstruct the outer pseudo-header
> (same as it does with the inner) and ignore any pre-seeded value in the
> checksum field. I'd expected other vendors to have gone down the same
> route, but if Intel didn't then maybe others didn't either. It'd be
> nice if some of them would chime in and let us know what they want...
Right. I added Or to the Cc. Maybe we can get some input from
Mellanox on how they do TSO and what changes they would need in order
to support TSO for GRE or UDP tunnels with checksum.
My concern is that I am not sure how much value there is to add with
this type of code if the hardware is parsing headers. In the case of
most of the Intel parts you specify offsets for the network and
transport headers so it gives you some degree of flexibility. If
however the hardware parses headers it becomes problematic as we can
only support protocols that can be parsed by the hardware. So for
example on the Intel parts using the drivers igb and ixgbe I will be
able to support VXLAN-GPE using a partial GSO approach. However for
fm10k which does some parsing for the tunnel headers I probably won't
as it will not know what to do if there are any extra headers included
and won't be able to determine the offset of the inner transport
header.
- Alex
^ permalink raw reply
* [PATCH 3/3] led: at8030: Add LED driver for AT8030 ethernet PHY
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
In-Reply-To: <1458755500-15571-1-git-send-email-vishalthanki@gmail.com>
This patch adds support for controlling the Link and
Activity LED of AT8030 ethernet PHY through LED subsystem.
Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
drivers/leds/Kconfig | 7 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-at803x.c | 158 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 166 insertions(+)
create mode 100644 drivers/leds/leds-at803x.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1f64151..07781ac 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -615,6 +615,13 @@ config LEDS_VERSATILE
This option enabled support for the LEDs on the ARM Versatile
and RealView boards. Say Y to enabled these.
+config LEDS_AT803X
+ tristate "LED support for the AT803X Ethernet PHY (Experimental)"
+ depends on LEDS_CLASS
+ help
+ This option enabled support for the LEDs on AT803X ethernet PHY.
+
+
comment "LED Triggers"
source "drivers/leds/trigger/Kconfig"
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index cb2013d..2d1ae65 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o
obj-$(CONFIG_LEDS_SEAD3) += leds-sead3.o
obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o
+obj-$(CONFIG_LEDS_AT803X) += leds-at803x.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff --git a/drivers/leds/leds-at803x.c b/drivers/leds/leds-at803x.c
new file mode 100644
index 0000000..58eb0c4
--- /dev/null
+++ b/drivers/leds/leds-at803x.c
@@ -0,0 +1,158 @@
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/leds.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/phy/at803x.h>
+
+#define LED_TX_NORMAL 0x0
+#define LED_TX_BLINK 0x1
+#define LED_TX_OFF 0x2
+#define LED_TX_ON 0x3
+
+#define LED_RX_NORMAL 0x0
+#define LED_RX_BLINK 0x1
+#define LED_RX_OFF 0x2
+#define LED_RX_ON 0x3
+
+#define LED_ACT_BLINK 0x1
+
+#define LED_LINK_NORMAL 0x0
+#define LED_LINK_BLINK LED_ACT_BLINK
+#define LED_LINK_OFF 0x2
+#define LED_LINK_ON 0x3
+
+#define LED_ACT_ACTIVE 0x0
+#define LED_ACT_LINK 0x1
+
+static void at803x_led_work(struct work_struct *work)
+{
+ struct at803x_phy_led *led =
+ container_of(work, struct at803x_phy_led, work);
+ union at803x_led_manual_ctrl regval = led->regval;
+
+ phy_write(led->led_grp->phydev, led->reg, regval.value);
+}
+
+static int at803x_blink_set(struct led_classdev *cdev,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ struct at803x_phy_led *led =
+ container_of(cdev, struct at803x_phy_led, cdev);
+ union at803x_led_manual_ctrl regval = {.value = 0};
+ int id = led->id;
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ switch (id) {
+ case AT803X_LINK:
+ /* Can't configure LED_LINK to blink */
+ break;
+ case AT803X_ACT:
+ /*
+ * LED will be ON when link is established
+ * and it will blink when link is active.
+ * delay_on/delay_off values are ignored.
+ */
+ regval.field.led_act_ctrl = LED_ACT_LINK;
+ break;
+ }
+ led->regval = regval;
+
+ schedule_work(&led->work);
+ spin_unlock_irqrestore(&led->lock, flags);
+ return 0;
+}
+
+
+static void at803x_phy_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct at803x_phy_led *led =
+ container_of(led_cdev, struct at803x_phy_led, cdev);
+ union at803x_led_manual_ctrl regval = led->regval;
+ int id = led->id;
+ unsigned long flags;
+
+ spin_lock_irqsave(&led->lock, flags);
+ switch (id) {
+ case AT803X_LINK:
+ regval.field.led_lnk_10_100_ctrl = value == LED_FULL ?
+ LED_LINK_ON : LED_LINK_OFF;
+ break;
+ case AT803X_ACT:
+ /* LED_ACT status is controlled via LED_TX and LED_RX */
+ regval.field.led_tx = value == LED_FULL ?
+ LED_TX_ON : LED_TX_OFF;
+ regval.field.led_rx = value == LED_FULL ?
+ LED_RX_ON : LED_RX_OFF;
+ break;
+ }
+
+ led->value = value;
+ led->regval = regval;
+ schedule_work(&led->work);
+ spin_unlock_irqrestore(&led->lock, flags);
+}
+
+static int at803x_phy_led_probe(struct platform_device *pdev)
+{
+ int i;
+ int ret;
+ struct at803x_phy_leds *led_grp = platform_get_drvdata(pdev);
+ struct at803x_phy_led *leds = led_grp->leds;
+
+ for (i = 0; i < led_grp->nr_leds; i++) {
+
+ leds[i].cdev.brightness_set = at803x_phy_led_brightness_set;
+ leds[i].cdev.blink_set = at803x_blink_set;
+
+ ret = led_classdev_register(&pdev->dev, &leds[i].cdev);
+ if (ret < 0)
+ goto err;
+
+ leds[i].led_grp = led_grp;
+ leds[i].regval.value = phy_read(led_grp->phydev, leds[i].reg);
+ INIT_WORK(&leds[i].work, at803x_led_work);
+ spin_lock_init(&leds[i].lock);
+
+ }
+
+ return 0;
+
+err:
+ for (i = i - 1; i >= 0; i--)
+ led_classdev_unregister(&leds[i].cdev);
+
+ return ret;
+}
+
+static int at803x_phy_led_remove(struct platform_device *pdev)
+{
+ int i;
+ struct at803x_phy_leds *led_grp = platform_get_drvdata(pdev);
+ struct at803x_phy_led *leds = led_grp->leds;
+
+
+ for (i = 0; i < led_grp->nr_leds; i++)
+ led_classdev_unregister(&leds[i].cdev);
+
+ return 0;
+}
+
+static struct platform_driver at803x_phy_led_driver = {
+ .probe = at803x_phy_led_probe,
+ .remove = at803x_phy_led_remove,
+ .driver = {
+ .name = "at803x-led",
+ },
+};
+
+module_platform_driver(at803x_phy_led_driver);
+
+MODULE_DESCRIPTION("at803x PHY LED driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-at803x");
--
2.4.3
^ permalink raw reply related
* [PATCH 2/3] net: phy: at8030: Expose the Link and Activity LEDs
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
In-Reply-To: <1458755500-15571-1-git-send-email-vishalthanki@gmail.com>
Expose these LEDs by means of a platform device which
can be controlled by a suitable LED driver.
Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
drivers/net/phy/at803x.c | 55 +++++++++++++++++++++++++++++++++++++++++++++-
include/linux/phy/at803x.h | 45 +++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 include/linux/phy/at803x.h
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 1e901c7..8fdb5ff 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -18,6 +18,8 @@
#include <linux/etherdevice.h>
#include <linux/of_gpio.h>
#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/phy/at803x.h>
#define AT803X_INTR_ENABLE 0x12
#define AT803X_INTR_ENABLE_AUTONEG_ERR BIT(15)
@@ -34,6 +36,7 @@
#define AT803X_SMART_SPEED 0x14
#define AT803X_LED_CONTROL 0x18
+#define AT803X_LED_MANUAL_CTRL 0x19
#define AT803X_DEVICE_ADDR 0x03
#define AT803X_LOC_MAC_ADDR_0_15_OFFSET 0x804C
@@ -117,6 +120,46 @@ static inline int at803x_enable_tx_delay(struct phy_device *phydev)
AT803X_DEBUG_TX_CLK_DLY_EN);
}
+static struct at803x_phy_led leds[] = {
+ {
+ .cdev.name = "at803x-link",
+ .id = AT803X_LINK,
+ .reg = AT803X_LED_MANUAL_CTRL,
+ },
+ {
+ .cdev.name = "at803x-act",
+ .id = AT803X_ACT,
+ .reg = AT803X_LED_MANUAL_CTRL,
+ },
+};
+
+static struct at803x_phy_leds led_grp = {
+ .leds = leds,
+ .nr_leds = ARRAY_SIZE(leds),
+};
+
+static struct platform_device *pdev;
+
+static int at803x_led_init(struct phy_device *phydev)
+{
+ led_grp.phydev = phydev;
+
+ pdev = platform_device_alloc("at803x-led", 0);
+ if (!pdev) {
+ pr_info("Failed to allocate LED\n");
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(pdev, &led_grp);
+
+ return platform_device_add(pdev);
+}
+
+static void at803x_led_cleanup(struct phy_device *phydev)
+{
+ platform_device_del(pdev);
+}
+
/* save relevant PHY registers to private copy */
static void at803x_context_save(struct phy_device *phydev,
struct at803x_context *context)
@@ -285,9 +328,18 @@ static int at803x_probe(struct phy_device *phydev)
phydev->priv = priv;
+ if (phydev->drv->flags & PHY_HAS_LED_CTRL)
+ return at803x_led_init(phydev);
+
return 0;
}
+static void at803x_remove(struct phy_device *phydev)
+{
+ if (phydev->drv->flags & PHY_HAS_LED_CTRL)
+ at803x_led_cleanup(phydev);
+}
+
static int at803x_config_init(struct phy_device *phydev)
{
int ret;
@@ -411,11 +463,12 @@ static struct phy_driver at803x_driver[] = {
.suspend = at803x_suspend,
.resume = at803x_resume,
.features = PHY_BASIC_FEATURES,
- .flags = PHY_HAS_INTERRUPT,
+ .flags = PHY_HAS_INTERRUPT | PHY_HAS_LED_CTRL,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = at803x_ack_interrupt,
.config_intr = at803x_config_intr,
+ .remove = at803x_remove,
}, {
/* ATHEROS 8031 */
.phy_id = ATH8031_PHY_ID,
diff --git a/include/linux/phy/at803x.h b/include/linux/phy/at803x.h
new file mode 100644
index 0000000..f33bfcf
--- /dev/null
+++ b/include/linux/phy/at803x.h
@@ -0,0 +1,45 @@
+#ifndef _AT803X_H
+#define _AT803X_H
+
+#include <linux/leds.h>
+#include <linux/phy/phy.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
+
+enum at803x_led_type {
+ AT803X_LINK,
+ AT803X_ACT,
+};
+
+union at803x_led_manual_ctrl {
+ struct {
+ u16 led_tx:2;
+ u16 led_rx:2;
+ u16 res3:2;
+ u16 led_lnk_10_100_ctrl:2;
+ u16 res2:4;
+ u16 led_act_ctrl:1;
+ u16 res1:3;
+
+ } field;
+ u16 value;
+};
+
+struct at803x_phy_led {
+ struct work_struct work;
+ struct led_classdev cdev;
+ enum led_brightness value;
+ union at803x_led_manual_ctrl regval;
+ enum at803x_led_type id;
+ struct at803x_phy_leds *led_grp;
+ spinlock_t lock;
+ u16 reg;
+};
+
+struct at803x_phy_leds {
+ struct phy_device *phydev;
+ struct at803x_phy_led *leds;
+ int nr_leds;
+};
+
+#endif
--
2.4.3
^ permalink raw reply related
* [PATCH 1/3] net: phy: Add ethernet PHY LED triggers
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
In-Reply-To: <1458755500-15571-1-git-send-email-vishalthanki@gmail.com>
Add the LED triggers for ethernet PHY link and activity
status. The triggers are set mainly based on the
PHY state machine.
Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
drivers/net/phy/Kconfig | 7 +++++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/phy.c | 20 +++++++++++--
drivers/net/phy/phy_device.c | 4 +++
drivers/net/phy/phy_led.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
drivers/net/phy/phy_led.h | 37 +++++++++++++++++++++++
include/linux/leds.h | 1 +
include/linux/phy.h | 6 ++++
8 files changed, 144 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/phy/phy_led.c
create mode 100644 drivers/net/phy/phy_led.h
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 6dad9a9..15712da 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -271,6 +271,13 @@ config MDIO_BCM_IPROC
This module provides a driver for the MDIO busses found in the
Broadcom iProc SoC's.
+config ETH_PHY_LED
+ bool "Ethernet PHY LED trigger support (Experimental)"
+ depends on LEDS_TRIGGERS
+ help
+ This feature enables the Ethernet PHY LED triggers for PHY link
+ and data transfer activities.
+
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index fcdbb92..76ebd83 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o
obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
obj-$(CONFIG_MICROCHIP_PHY) += microchip.o
obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
+obj-$(CONFIG_ETH_PHY_LED) += phy_led.o
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 5590b9c..e422ff6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -35,6 +35,7 @@
#include <linux/io.h>
#include <linux/uaccess.h>
#include <linux/atomic.h>
+#include "phy_led.h"
#include <asm/irq.h>
@@ -831,15 +832,15 @@ void phy_state_machine(struct work_struct *work)
switch (phydev->state) {
case PHY_DOWN:
+ phy_trig_led_link(phydev, 0);
case PHY_STARTING:
case PHY_READY:
case PHY_PENDING:
break;
case PHY_UP:
needs_aneg = true;
-
phydev->link_timeout = PHY_AN_TIMEOUT;
-
+ phy_trig_led_link(phydev, 1);
break;
case PHY_AN:
err = phy_read_status(phydev);
@@ -849,6 +850,7 @@ void phy_state_machine(struct work_struct *work)
/* If the link is down, give up on negotiation for now */
if (!phydev->link) {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
netif_carrier_off(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
break;
@@ -862,6 +864,7 @@ void phy_state_machine(struct work_struct *work)
/* If AN is done, we're running */
if (err > 0) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
@@ -889,6 +892,7 @@ void phy_state_machine(struct work_struct *work)
}
}
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
}
@@ -900,6 +904,8 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
+ phy_trig_led_link(phydev, 1);
netif_carrier_on(phydev->attached_dev);
} else {
if (0 == phydev->link_timeout--)
@@ -929,9 +935,12 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_act(phydev);
+ phy_trig_led_link(phydev, 1);
netif_carrier_on(phydev->attached_dev);
} else {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
netif_carrier_off(phydev->attached_dev);
}
@@ -948,6 +957,7 @@ void phy_state_machine(struct work_struct *work)
phydev->adjust_link(phydev->attached_dev);
do_suspend = true;
}
+ led_trigger_event(phydev->phy_act, LED_OFF);
break;
case PHY_RESUMING:
if (AUTONEG_ENABLE == phydev->autoneg) {
@@ -965,9 +975,12 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_link(phydev, 1);
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
} else {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
}
phydev->adjust_link(phydev->attached_dev);
} else {
@@ -981,9 +994,12 @@ void phy_state_machine(struct work_struct *work)
if (phydev->link) {
phydev->state = PHY_RUNNING;
+ phy_trig_led_link(phydev, 1);
+ phy_trig_led_act(phydev);
netif_carrier_on(phydev->attached_dev);
} else {
phydev->state = PHY_NOLINK;
+ phy_trig_led_link(phydev, 0);
}
phydev->adjust_link(phydev->attached_dev);
}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e551f3a..fafcc3c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -34,6 +34,7 @@
#include <linux/io.h>
#include <linux/uaccess.h>
#include <linux/of.h>
+#include "phy_led.h"
#include <asm/irq.h>
@@ -1592,6 +1593,8 @@ static int phy_probe(struct device *dev)
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
+ phy_led_init(phydev);
+
/* Set the state to READY by default */
phydev->state = PHY_READY;
@@ -1608,6 +1611,7 @@ static int phy_remove(struct device *dev)
struct phy_device *phydev = to_phy_device(dev);
mutex_lock(&phydev->lock);
+ phy_led_cleanup(phydev);
phydev->state = PHY_DOWN;
mutex_unlock(&phydev->lock);
diff --git a/drivers/net/phy/phy_led.c b/drivers/net/phy/phy_led.c
new file mode 100644
index 0000000..240355a
--- /dev/null
+++ b/drivers/net/phy/phy_led.c
@@ -0,0 +1,70 @@
+#include <linux/phy.h>
+#include <linux/leds.h>
+
+static void phy_link_trig_activate(struct led_classdev *led)
+{
+ struct phy_device *phydev = led->trigger->data;
+ enum led_brightness value = LED_OFF;
+
+ if (phydev && phydev->state != PHY_DOWN)
+ value = LED_FULL;
+ led_set_brightness(led, value);
+}
+
+static void phy_act_trig_activate(struct led_classdev *led)
+{
+ struct phy_device *phydev = led->trigger->data;
+ unsigned long on = 0, off = 0;
+
+ if (phydev && (phydev->state == PHY_RUNNING ||
+ phydev->state == PHY_CHANGELINK))
+ led_blink_set(led, &on, &off);
+}
+
+void phy_led_init(struct phy_device *phydev)
+{
+ phydev->phy_link = kzalloc(sizeof(*phydev->phy_link), GFP_KERNEL);
+ if (!phydev->phy_link)
+ return;
+
+ snprintf(phydev->phy_link_name, sizeof(phydev->phy_link_name),
+ "%x-eth-phy-link", phydev->drv->phy_id);
+ phydev->phy_link->name = phydev->phy_link_name;
+ if (led_trigger_register(phydev->phy_link)) {
+ kfree(phydev->phy_link);
+ phydev->phy_link = NULL;
+ }
+ phydev->phy_link->data = phydev;
+ phydev->phy_link->activate = phy_link_trig_activate;
+
+ phydev->phy_act = kzalloc(sizeof(*phydev->phy_act), GFP_KERNEL);
+ if (!phydev->phy_act) {
+ led_trigger_unregister(phydev->phy_link);
+ kfree(phydev->phy_link);
+ return;
+ }
+ phydev->phy_act->data = phydev;
+ phydev->phy_act->activate = phy_act_trig_activate;
+
+ snprintf(phydev->phy_act_name, sizeof(phydev->phy_act_name),
+ "%x-eth-phy-activity", phydev->drv->phy_id);
+ phydev->phy_act->name = phydev->phy_act_name;
+ if (led_trigger_register(phydev->phy_act)) {
+ kfree(phydev->phy_act);
+ phydev->phy_act = NULL;
+ return;
+ }
+}
+
+void phy_led_cleanup(struct phy_device *phydev)
+{
+ if (phydev->phy_link) {
+ led_trigger_unregister(phydev->phy_link);
+ kfree(phydev->phy_link);
+ }
+ if (phydev->phy_act) {
+ led_trigger_unregister(phydev->phy_act);
+ kfree(phydev->phy_act);
+ }
+}
+
diff --git a/drivers/net/phy/phy_led.h b/drivers/net/phy/phy_led.h
new file mode 100644
index 0000000..ff5bd98
--- /dev/null
+++ b/drivers/net/phy/phy_led.h
@@ -0,0 +1,37 @@
+#include <linux/leds.h>
+
+#ifdef CONFIG_ETH_PHY_LED
+static inline void phy_trig_led_act(struct phy_device *phydev)
+{
+ unsigned long on = 0, off = 0;
+
+ led_trigger_blink(phydev->phy_act, &on, &off);
+}
+
+static inline void phy_trig_led_link(struct phy_device *phydev, int on)
+{
+ enum led_brightness value = (on) ? LED_FULL : LED_OFF;
+
+ led_trigger_event(phydev->phy_link, value);
+}
+
+void phy_led_init(struct phy_device *phydev);
+void phy_led_cleanup(struct phy_device *phydev);
+#else
+static inline void phy_trig_led_act(struct phy_device *phydev)
+{
+}
+
+static inline void phy_trig_led_link(struct phy_device *phydev, int on)
+{
+}
+
+static inline void phy_led_init(struct phy_device *phydev)
+{
+}
+
+static inline void phy_led_cleanup(struct phy_device *phydev)
+{
+}
+#endif
+
diff --git a/include/linux/leds.h b/include/linux/leds.h
index f203a8f..11ec574 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -233,6 +233,7 @@ struct led_trigger {
const char *name;
void (*activate)(struct led_classdev *led_cdev);
void (*deactivate)(struct led_classdev *led_cdev);
+ void *data;
/* LEDs under control by this trigger (for simple triggers) */
rwlock_t leddev_list_lock;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2abd791..94fee77 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -25,6 +25,7 @@
#include <linux/timer.h>
#include <linux/workqueue.h>
#include <linux/mod_devicetable.h>
+#include <linux/leds.h>
#include <linux/atomic.h>
@@ -60,6 +61,7 @@
#define PHY_HAS_INTERRUPT 0x00000001
#define PHY_HAS_MAGICANEG 0x00000002
#define PHY_IS_INTERNAL 0x00000004
+#define PHY_HAS_LED_CTRL 0x00000008
#define MDIO_DEVICE_IS_PHY 0x80000000
/* Interface Mode definitions */
@@ -424,6 +426,10 @@ struct phy_device {
u8 mdix;
void (*adjust_link)(struct net_device *dev);
+
+ struct led_trigger *phy_link, *phy_act;
+ char phy_link_name[32], phy_act_name[32];
+
};
#define to_phy_device(d) container_of(to_mdio_device(d), \
struct phy_device, mdio)
--
2.4.3
^ permalink raw reply related
* [PATCH 0/3] Control ethernet PHY LEDs via LED subsystem
From: Vishal Thanki @ 2016-03-23 17:51 UTC (permalink / raw)
To: andrew, f.fainelli, ujhelyi.m, netdev; +Cc: Vishal Thanki
Hi all,
Based on suggestions from Andrew and Florian, I have made some changes
to expose the ethernet PHY LEDs using kernel LED subsystem. The following
ALPHA patchset introduces two new LED triggers:
1) <phydev>-eth-phy-link:
To monitor the PHY Link status
2) <phydev>-eth-phy-activity:
To monitor the PHY activity status. This trigger may still some more work
because as of now it just takes decision to set the trigger based on
PHY state machine and triggers the blink_set with delay_on and delay_off
parameters set to 0.
Please provide the review comments so that I can work on this patchset to
make it complete.
Thanks,
Vishal
Vishal Thanki (3):
net: phy: Add ethernet PHY LED triggers
net: phy: at8030: Expose the Link and Activity LEDs
led: at8030: Add LED driver for AT8030 ethernet PHY
drivers/leds/Kconfig | 7 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-at803x.c | 158 +++++++++++++++++++++++++++++++++++++++++++
drivers/net/phy/Kconfig | 7 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/at803x.c | 55 ++++++++++++++-
drivers/net/phy/phy.c | 20 +++++-
drivers/net/phy/phy_device.c | 4 ++
drivers/net/phy/phy_led.c | 70 +++++++++++++++++++
drivers/net/phy/phy_led.h | 37 ++++++++++
include/linux/leds.h | 1 +
include/linux/phy.h | 6 ++
include/linux/phy/at803x.h | 45 ++++++++++++
13 files changed, 409 insertions(+), 3 deletions(-)
create mode 100644 drivers/leds/leds-at803x.c
create mode 100644 drivers/net/phy/phy_led.c
create mode 100644 drivers/net/phy/phy_led.h
create mode 100644 include/linux/phy/at803x.h
--
2.4.3
^ permalink raw reply
* Re: [PATCH] net: mediatek: fix checking for NULL instead of IS_ERR() in .probe
From: David Miller @ 2016-03-23 17:50 UTC (permalink / raw)
To: vz; +Cc: nbd, blogic, netdev, linux-arm-kernel, linux-mediatek
In-Reply-To: <1458687964-9690-1-git-send-email-vz@mleia.com>
From: Vladimir Zapolskiy <vz@mleia.com>
Date: Wed, 23 Mar 2016 01:06:04 +0200
> devm_ioremap_resource() returns ERR_PTR() value on error, it never
> returns NULL, fix it and propagate the returned error upwards.
>
> Fixes: 656e705243fd ("net-next: mediatek: add support for MT7623 ethernet")
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/2] net: ethernet: dlink: dl2k: fix code style
From: David Miller @ 2016-03-23 17:41 UTC (permalink / raw)
To: mussitantesmortem; +Cc: linux, netdev, linux-kernel
In-Reply-To: <1458743800-30606-1-git-send-email-mussitantesmortem@gmail.com>
Such cleanups and tidies are not appropriate to be submitted at this time.
Only bug fixes should be going into the tree right now.
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: Request 'reset' GPIO only for AT8030 PHY
From: David Miller @ 2016-03-23 17:40 UTC (permalink / raw)
To: sf84
Cc: u.kleine-koenig, daniel, netdev, linux-kernel, slash.tmp,
f.fainelli, mans, festevam, martin.blumenstingl, linus.walleij
In-Reply-To: <56F274A5.6030502@laposte.net>
From: Sebastian Frias <sf84@laposte.net>
Date: Wed, 23 Mar 2016 11:49:09 +0100
> This removes the dependency on GPIOLIB for non faulty PHYs.
>
> Indeed, without this patch, if GPIOLIB is not selected
> devm_gpiod_get_optional() will return -ENOSYS and the driver probe
> call will fail, regardless of the actual PHY hardware.
>
> Out of the 3 PHYs supported by this driver (AT8030, AT8031, AT8035),
> only AT8030 presents the issues that commit 13a56b449325 ("net: phy:
> at803x: Add support for hardware reset") attempts to work-around by
> using a 'reset' GPIO line.
>
> Hence, only AT8030 should depend on GPIOLIB operating properly.
>
> Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
Applied.
^ permalink raw reply
* Re: [PATCH] at803x: fix reset handling
From: David Miller @ 2016-03-23 17:40 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, f.fainelli, u.kleine-koenig
In-Reply-To: <1525241.UQIRf9ZOB3@wasted.cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 23 Mar 2016 00:44:40 +0300
> The driver of course "knows" that the chip's reset signal is active low,
> so it drives the GPIO to 0 to reset the PHY and to 1 otherwise; however
> all this will only work iff the GPIO is specified as active-high in the
> device tree! I think both the driver and the device trees (if there are
> any -- I was unable to find them) need to be fixed in this case...
>
> Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Applied.
^ permalink raw reply
* Re: net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Pablo Neira Ayuso @ 2016-03-23 17:38 UTC (permalink / raw)
To: Baozeng
Cc: Eric Dumazet, kaber, kadlec, coreteam, netfilter-devel,
Vladislav Yasevich, nhorman, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CAP=GMUFA-qAOxF1W=aeWDYBiR-GSohJYGHiMDsrqwsD0xdg3_Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 998 bytes --]
On Thu, Mar 24, 2016 at 12:42:43AM +0800, Baozeng wrote:
> 2016-03-22 23:27 GMT+08:00 Eric Dumazet <eric.dumazet@gmail.com>:
> > Untested patch would be :
> >
> > diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> > index 67b2e27999aa..fceb7354d169 100644
> > --- a/net/bridge/netfilter/ebtables.c
> > +++ b/net/bridge/netfilter/ebtables.c
> > @@ -346,7 +346,7 @@ find_inlist_lock(struct list_head *head, const char *name, const char *prefix,
> > {
> > return try_then_request_module(
> > find_inlist_lock_noload(head, name, error, mutex),
> > - "%s%s", prefix, name);
> > + "%.*s%s", EBT_TABLE_MAXNAMELEN, prefix, name);
> > }
> >
> > static inline struct ebt_table *
> >
> >
>
> Thanks for your quick patch. I tested it but it still reproduce the
> bug. We should limit the length of the name,
> not the prefix. The following patch fixs it.
Could you give a try to this patch? Thanks.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2686 bytes --]
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 67b2e27..1e3a707 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1521,6 +1521,8 @@ static int do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
if (copy_from_user(&tmp, user, sizeof(tmp)))
return -EFAULT;
+ tmp.name[sizeof(tmp.name) - 1] = '\0';
+
t = find_table_lock(net, tmp.name, &ret, &ebt_mutex);
if (!t)
return ret;
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index a2002ff..06e9fb7 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -960,6 +960,7 @@ static int get_entries(struct net *net, struct arpt_get_entries __user *uptr,
sizeof(struct arpt_get_entries) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
if (!IS_ERR_OR_NULL(t)) {
@@ -1654,6 +1655,7 @@ static int compat_get_entries(struct net *net,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
xt_compat_lock(NFPROTO_ARP);
t = xt_find_table_lock(net, NFPROTO_ARP, get.name);
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 45b1d97..4642dc2 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1146,6 +1146,7 @@ get_entries(struct net *net, struct ipt_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
t = xt_find_table_lock(net, AF_INET, get.name);
if (!IS_ERR_OR_NULL(t)) {
@@ -1925,6 +1926,7 @@ compat_get_entries(struct net *net, struct compat_ipt_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
xt_compat_lock(AF_INET);
t = xt_find_table_lock(net, AF_INET, get.name);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 85c0942..db47815 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1158,6 +1158,7 @@ get_entries(struct net *net, struct ip6t_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
t = xt_find_table_lock(net, AF_INET6, get.name);
if (!IS_ERR_OR_NULL(t)) {
@@ -1934,6 +1935,7 @@ compat_get_entries(struct net *net, struct compat_ip6t_get_entries __user *uptr,
*len, sizeof(get) + get.size);
return -EINVAL;
}
+ get.name[sizeof(get.name) - 1] = '\0';
xt_compat_lock(AF_INET6);
t = xt_find_table_lock(net, AF_INET6, get.name);
^ permalink raw reply related
* Re: [PATCH 2/2] drivers: net: ethernet: dlink: dl2k: fix indent
From: Joe Perches @ 2016-03-23 17:34 UTC (permalink / raw)
To: Maxim Zhukov, davem; +Cc: linux, netdev, linux-kernel
In-Reply-To: <1458748660-10571-2-git-send-email-mussitantesmortem@gmail.com>
If dl2k is modified at all, maybe convert the
printks to netdev_<level> too so that the logging
output is more like other networking drivers.
Something like:
---
drivers/net/ethernet/dlink/dl2k.c | 181 +++++++++++++++++++++-----------------
1 file changed, 100 insertions(+), 81 deletions(-)
diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index f92b6d9..0f0326b 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -10,9 +10,9 @@
(at your option) any later version.
*/
-#define DRV_NAME "DL2000/TC902x-based linux driver"
#define DRV_VERSION "v1.19"
#define DRV_RELDATE "2007/08/12"
+
#include "dl2k.h"
#include <linux/dma-mapping.h>
@@ -23,8 +23,6 @@
#define dr16(reg) ioread16(ioaddr + (reg))
#define dr8(reg) ioread8(ioaddr + (reg))
-static char version[] =
- KERN_INFO DRV_NAME " " DRV_VERSION " " DRV_RELDATE "\n";
#define MAX_UNITS 8
static int mtu[MAX_UNITS];
static int vlan[MAX_UNITS];
@@ -118,12 +116,11 @@ rio_probe1 (struct pci_dev *pdev, const struct pci_device_id *ent)
int chip_idx = ent->driver_data;
int err, irq;
void __iomem *ioaddr;
- static int version_printed;
void *ring_space;
dma_addr_t ring_dma;
- if (!version_printed++)
- printk ("%s", version);
+ pr_info_once("DL2000/TC902x-based linux driver %s %s\n",
+ DRV_VERSION, DRV_RELDATE);
err = pci_enable_device (pdev);
if (err)
@@ -274,18 +271,20 @@ rio_probe1 (struct pci_dev *pdev, const struct pci_device_id *ent)
card_idx++;
- printk (KERN_INFO "%s: %s, %pM, IRQ %d\n",
- dev->name, np->name, dev->dev_addr, irq);
+ netdev_info(dev, "%s, %pM, IRQ %d\n",
+ np->name, dev->dev_addr, irq);
if (tx_coalesce > 1)
- printk(KERN_INFO "tx_coalesce:\t%d packets\n",
- tx_coalesce);
- if (np->coalesce)
- printk(KERN_INFO
- "rx_coalesce:\t%d packets\n"
- "rx_timeout: \t%d ns\n",
- np->rx_coalesce, np->rx_timeout*640);
+ netdev_info(dev, "tx_coalesce:\t%d packets\n",
+ tx_coalesce);
+ if (np->coalesce) {
+ netdev_info(dev, "rx_coalesce:\t%d packets\n",
+ np->rx_coalesce);
+ netdev_info(dev, "rx_timeout: \t%d ns\n",
+ np->rx_timeout * 640);
+ }
if (np->vlan)
- printk(KERN_INFO "vlan(id):\t%d\n", np->vlan);
+ netdev_info(dev, "vlan(id):\t%d\n", np->vlan);
+
return 0;
err_out_unmap_rx:
@@ -322,7 +321,7 @@ find_miiphy (struct net_device *dev)
}
}
if (!phy_found) {
- printk (KERN_ERR "%s: No MII PHY found!\n", dev->name);
+ netdev_err(dev, "No MII PHY found!\n");
return -ENODEV;
}
return 0;
@@ -348,8 +347,7 @@ parse_eeprom (struct net_device *dev)
/* Check CRC */
crc = ~ether_crc_le (256 - 4, sromdata);
if (psrom->crc != cpu_to_le32(crc)) {
- printk (KERN_ERR "%s: EEPROM data CRC error.\n",
- dev->name);
+ netdev_err(dev, "EEPROM data CRC error\n");
return -1;
}
}
@@ -374,7 +372,7 @@ parse_eeprom (struct net_device *dev)
cid = psib[i++];
next = psib[i++];
if ((cid == 0 && next == 0) || (cid == 0xff && next == 0xff)) {
- printk (KERN_ERR "Cell data error\n");
+ netdev_err(dev, "Cell data error\n");
return -1;
}
switch (cid) {
@@ -664,7 +662,7 @@ rio_timer (unsigned long data)
spin_lock_irqsave(&np->rx_lock, flags);
/* Recover rx ring exhausted error */
if (np->cur_rx - np->old_rx >= RX_RING_SIZE) {
- printk(KERN_INFO "Try to recover rx ring exhausted...\n");
+ netdev_info(dev, "Trying to recover, rx ring exhausted...\n");
/* Re-allocate skbuffs to fill the descriptor ring */
for (; np->cur_rx - np->old_rx > 0; np->old_rx++) {
struct sk_buff *skb;
@@ -675,9 +673,8 @@ rio_timer (unsigned long data)
np->rx_buf_sz);
if (skb == NULL) {
np->rx_ring[entry].fraginfo = 0;
- printk (KERN_INFO
- "%s: Still unable to re-allocate Rx skbuff.#%d\n",
- dev->name, entry);
+ netdev_info(dev, "Still unable to re-allocate Rx skbuff.#%d\n",
+ entry);
break;
}
np->rx_skbuff[entry] = skb;
@@ -702,8 +699,8 @@ rio_tx_timeout (struct net_device *dev)
struct netdev_private *np = netdev_priv(dev);
void __iomem *ioaddr = np->ioaddr;
- printk (KERN_INFO "%s: Tx timed out (%4.4x), is buffer full?\n",
- dev->name, dr32(TxStatus));
+ netdev_info(dev, "Tx timed out (%04x), is buffer full?\n",
+ dr32(TxStatus));
rio_free_tx(dev, 0);
dev->if_port = 0;
dev->trans_start = jiffies; /* prevent tx timeout */
@@ -874,8 +871,8 @@ tx_error (struct net_device *dev, int tx_status)
int i;
frame_id = (tx_status & 0xffff0000);
- printk (KERN_ERR "%s: Transmit error, TxStatus %4.4x, FrameId %d.\n",
- dev->name, tx_status, frame_id);
+ netdev_err(dev, "Transmit error, TxStatus %04x, FrameId %d\n",
+ tx_status, frame_id);
np->stats.tx_errors++;
/* Ttransmit Underrun */
if (tx_status & 0x10) {
@@ -1007,10 +1004,8 @@ receive_packet (struct net_device *dev)
skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
if (skb == NULL) {
np->rx_ring[entry].fraginfo = 0;
- printk (KERN_INFO
- "%s: receive_packet: "
- "Unable to re-allocate Rx skbuff.#%d\n",
- dev->name, entry);
+ netdev_info(dev, "receive_packet: Unable to re-allocate Rx skbuff.#%d\n",
+ entry);
break;
}
np->rx_skbuff[entry] = skb;
@@ -1039,7 +1034,7 @@ rio_error (struct net_device *dev, int int_status)
/* Link change event */
if (int_status & LinkEvent) {
if (mii_wait_link (dev, 10) == 0) {
- printk (KERN_INFO "%s: Link up\n", dev->name);
+ netdev_info(dev, "Link up\n");
if (np->phy_media)
mii_get_media_pcs (dev);
else
@@ -1059,7 +1054,7 @@ rio_error (struct net_device *dev, int int_status)
np->link_status = 1;
netif_carrier_on(dev);
} else {
- printk (KERN_INFO "%s: Link off\n", dev->name);
+ netdev_info(dev, "Link off\n");
np->link_status = 0;
netif_carrier_off(dev);
}
@@ -1073,8 +1068,7 @@ rio_error (struct net_device *dev, int int_status)
/* PCI Error, a catastronphic error related to the bus interface
occurs, set GlobalReset and HostReset to reset. */
if (int_status & HostError) {
- printk (KERN_ERR "%s: HostError! IntStatus %4.4x.\n",
- dev->name, int_status);
+ netdev_err(dev, "HostError! IntStatus %04x\n", int_status);
dw16(ASICCtrl + 2, GlobalReset | HostReset);
mdelay (500);
rio_set_led_mode(dev);
@@ -1325,7 +1319,7 @@ static int rio_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
if (np->speed == 1000) {
ethtool_cmd_speed_set(cmd, SPEED_100);
cmd->duplex = DUPLEX_FULL;
- printk("Warning!! Can't disable Auto negotiation in 1000Mbps, change to Manual 100Mbps, Full duplex.\n");
+ netdev_warn(dev, "Can't disable Auto negotiation in 1000Mbps, change to Manual 100Mbps, Full duplex\n");
}
switch (ethtool_cmd_speed(cmd)) {
case SPEED_10:
@@ -1515,6 +1509,9 @@ mii_get_media (struct net_device *dev)
__u16 mssr;
int phy_addr;
struct netdev_private *np;
+ const char *how;
+ const char *speed;
+ const char *duplex;
np = netdev_priv(dev);
phy_addr = np->phy_addr;
@@ -1525,6 +1522,7 @@ mii_get_media (struct net_device *dev)
/* Auto-Negotiation not completed */
return -1;
}
+ how = "Auto";
negotiate = mii_read (dev, phy_addr, MII_ADVERTISE) &
mii_read (dev, phy_addr, MII_LPA);
mscr = mii_read (dev, phy_addr, MII_CTRL1000);
@@ -1532,27 +1530,36 @@ mii_get_media (struct net_device *dev)
if (mscr & ADVERTISE_1000FULL && mssr & LPA_1000FULL) {
np->speed = 1000;
np->full_duplex = 1;
- printk (KERN_INFO "Auto 1000 Mbps, Full duplex\n");
+ speed = "1000";
+ duplex = "Full";
} else if (mscr & ADVERTISE_1000HALF && mssr & LPA_1000HALF) {
np->speed = 1000;
np->full_duplex = 0;
- printk (KERN_INFO "Auto 1000 Mbps, Half duplex\n");
+ speed = "1000";
+ duplex = "Half";
} else if (negotiate & ADVERTISE_100FULL) {
np->speed = 100;
np->full_duplex = 1;
- printk (KERN_INFO "Auto 100 Mbps, Full duplex\n");
+ speed = "100";
+ duplex = "Full";
} else if (negotiate & ADVERTISE_100HALF) {
np->speed = 100;
np->full_duplex = 0;
- printk (KERN_INFO "Auto 100 Mbps, Half duplex\n");
+ speed = "100";
+ duplex = "Half";
} else if (negotiate & ADVERTISE_10FULL) {
np->speed = 10;
np->full_duplex = 1;
- printk (KERN_INFO "Auto 10 Mbps, Full duplex\n");
+ speed = "10";
+ duplex = "Full";
} else if (negotiate & ADVERTISE_10HALF) {
np->speed = 10;
np->full_duplex = 0;
- printk (KERN_INFO "Auto 10 Mbps, Half duplex\n");
+ speed = "10";
+ duplex = "Half";
+ } else {
+ speed = "unknown";
+ duplex = "unknown";
}
if (negotiate & ADVERTISE_PAUSE_CAP) {
np->tx_flow &= 1;
@@ -1564,30 +1571,33 @@ mii_get_media (struct net_device *dev)
/* else tx_flow, rx_flow = user select */
} else {
__u16 bmcr = mii_read (dev, phy_addr, MII_BMCR);
+
+ how = "Operating at";
switch (bmcr & (BMCR_SPEED100 | BMCR_SPEED1000)) {
case BMCR_SPEED1000:
- printk (KERN_INFO "Operating at 1000 Mbps, ");
+ speed = "1000";
break;
case BMCR_SPEED100:
- printk (KERN_INFO "Operating at 100 Mbps, ");
+ speed = "100";
break;
case 0:
- printk (KERN_INFO "Operating at 10 Mbps, ");
- }
- if (bmcr & BMCR_FULLDPLX) {
- printk (KERN_CONT "Full duplex\n");
- } else {
- printk (KERN_CONT "Half duplex\n");
+ speed = "10";
+ break;
+ default:
+ speed = "unknown";
+ break;
}
+ if (bmcr & BMCR_FULLDPLX)
+ duplex = "Full";
+ else
+ duplex = "Half";
+
}
- if (np->tx_flow)
- printk(KERN_INFO "Enable Tx Flow Control\n");
- else
- printk(KERN_INFO "Disable Tx Flow Control\n");
- if (np->rx_flow)
- printk(KERN_INFO "Enable Rx Flow Control\n");
- else
- printk(KERN_INFO "Disable Rx Flow Control\n");
+
+ netdev_info(dev, "%s %s Mbps, %s duplex, Tx Flow Control: %s Rx Flow Control: %s\n",
+ how, speed, duplex,
+ np->tx_flow ? "Enabled" : "Disabled",
+ np->rx_flow ? "Enabled" : "Disabled");
return 0;
}
@@ -1636,6 +1646,10 @@ mii_set_media (struct net_device *dev)
mii_write (dev, phy_addr, MII_BMCR, bmcr);
mdelay(1);
} else {
+ const char *how = "Manual";
+ const char *speed = "unknown";
+ const char *duplex = "unknown";
+
/* Force speed setting */
/* 1) Disable Auto crossover */
pscr = mii_read (dev, phy_addr, MII_PHY_SCR);
@@ -1659,16 +1673,18 @@ mii_set_media (struct net_device *dev)
bmcr = BMCR_PDOWN;
if (np->speed == 100) {
bmcr |= BMCR_SPEED100;
- printk (KERN_INFO "Manual 100 Mbps, ");
+ speed = "100";
} else if (np->speed == 10) {
- printk (KERN_INFO "Manual 10 Mbps, ");
+ speed = "10";
}
if (np->full_duplex) {
bmcr |= BMCR_FULLDPLX;
- printk (KERN_CONT "Full duplex\n");
+ duplex = "Full";
} else {
- printk (KERN_CONT "Half duplex\n");
+ duplex = "Half";
}
+ netdev_info(dev, "%s %s Mbps, %s duplex\n",
+ how, speed, duplex);
#if 0
/* Set 1000BaseT Master/Slave setting */
mscr = mii_read (dev, phy_addr, MII_CTRL1000);
@@ -1688,6 +1704,9 @@ mii_get_media_pcs (struct net_device *dev)
__u16 bmsr;
int phy_addr;
struct netdev_private *np;
+ const char *how;
+ const char *speed;
+ const char *duplex;
np = netdev_priv(dev);
phy_addr = np->phy_addr;
@@ -1698,15 +1717,17 @@ mii_get_media_pcs (struct net_device *dev)
/* Auto-Negotiation not completed */
return -1;
}
+ how = "Auto";
negotiate = mii_read (dev, phy_addr, PCS_ANAR) &
mii_read (dev, phy_addr, PCS_ANLPAR);
np->speed = 1000;
+ speed = "1000";
if (negotiate & PCS_ANAR_FULL_DUPLEX) {
- printk (KERN_INFO "Auto 1000 Mbps, Full duplex\n");
np->full_duplex = 1;
+ duplex = "Full";
} else {
- printk (KERN_INFO "Auto 1000 Mbps, half duplex\n");
np->full_duplex = 0;
+ duplex = "Half";
}
if (negotiate & PCS_ANAR_PAUSE) {
np->tx_flow &= 1;
@@ -1718,21 +1739,19 @@ mii_get_media_pcs (struct net_device *dev)
/* else tx_flow, rx_flow = user select */
} else {
__u16 bmcr = mii_read (dev, phy_addr, PCS_BMCR);
- printk (KERN_INFO "Operating at 1000 Mbps, ");
- if (bmcr & BMCR_FULLDPLX) {
- printk (KERN_CONT "Full duplex\n");
- } else {
- printk (KERN_CONT "Half duplex\n");
- }
+
+ how = "Operating at";
+ speed = "1000";
+ if (bmcr & BMCR_FULLDPLX)
+ duplex = "Full";
+ else
+ duplex = "Half";
}
- if (np->tx_flow)
- printk(KERN_INFO "Enable Tx Flow Control\n");
- else
- printk(KERN_INFO "Disable Tx Flow Control\n");
- if (np->rx_flow)
- printk(KERN_INFO "Enable Rx Flow Control\n");
- else
- printk(KERN_INFO "Disable Rx Flow Control\n");
+
+ netdev_info(dev, "%s %s Mbps, %s duplex, Tx Flow Control: %s Rx Flow Control: %s\n",
+ how, speed, duplex,
+ np->tx_flow ? "Enabled" : "Disabled",
+ np->rx_flow ? "Enabled" : "Disabled");
return 0;
}
@@ -1775,10 +1794,10 @@ mii_set_media_pcs (struct net_device *dev)
mdelay(10);
if (np->full_duplex) {
bmcr = BMCR_FULLDPLX;
- printk (KERN_INFO "Manual full duplex\n");
+ netdev_info(dev, "Manual full duplex\n");
} else {
bmcr = 0;
- printk (KERN_INFO "Manual half duplex\n");
+ netdev_info(dev, "Manual half duplex\n");
}
mii_write (dev, phy_addr, MII_BMCR, bmcr);
mdelay(10);
^ permalink raw reply related
* Re: [PATCH net v2] xfrm: Fix crash observed during device unregistration and decryption
From: Eric Dumazet @ 2016-03-23 17:29 UTC (permalink / raw)
To: Herbert Xu
Cc: Subash Abhinov Kasiviswanathan, 'Steffen Klassert',
netdev, jeromes
In-Reply-To: <20160323132947.GA29938@gondor.apana.org.au>
On Wed, 2016-03-23 at 21:29 +0800, Herbert Xu wrote:
> On Wed, Mar 23, 2016 at 06:25:47AM -0700, Eric Dumazet wrote:
> >
> > Wont this prevent device from being dismantled ?
>
> This is only held while the crypto processing is ongoing.
>
> > Where is this xfrm queue purged at device dismantle ?
>
> There is no way to cancel an ongoing crypto processing so you'll
> just have to wait it out.
OK, but before calling netif_rx() are we properly testing dev->flags
IFF_UP status ?
Otherwise, we still allow packets being queued after flush_backlog() had
been called.
Thanks.
^ permalink raw reply
* Re: [net PATCH] net: Reset encap_level to avoid resetting features on inner IP headers
From: Tom Herbert @ 2016-03-23 17:25 UTC (permalink / raw)
To: Alexander Duyck
Cc: Kernel Team, David S. Miller, Alexander Duyck,
Linux Kernel Network Developers
In-Reply-To: <20160322231732.13054.71337.stgit@localhost.localdomain>
On Tue, Mar 22, 2016 at 4:18 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch corrects an oversight in which we were allowing the encap_level
> value to pass from the outer headers to the inner headers. As a result we
> were incorrectly identifying UDP or GRE tunnels as also making use of ipip
> or sit when the second header actually represented a tunnel encapsulated in
> either a UDP or GRE tunnel which already had the features masked.
>
> Fixes: 76443456227097179c1482 ("net: Move GSO csum into SKB_GSO_CB")
> Reported-by: Tom Herbert <tom@herbertland.com>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Acked-by: Tom Herbert <tom@herbertland.com>
I do found it strange that we need to lie about the encap_level to get
things to work. Hopefully, once the remaining encapsulation devices
move to NETIF_HW_CSUM we can get rid of enc_features entirely.
> ---
> net/ipv4/gre_offload.c | 1 +
> net/ipv4/udp_offload.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index dea0390d65bb..43a5c76742dc 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -49,6 +49,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>
> /* setup inner skb. */
> skb->encapsulation = 0;
> + SKB_GSO_CB(skb)->encap_level = 0;
> __skb_pull(skb, tnl_hlen);
> skb_reset_mac_header(skb);
> skb_set_network_header(skb, skb_inner_network_offset(skb));
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 5fcb93269afb..80236a9e1769 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -56,6 +56,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
>
> /* setup inner skb. */
> skb->encapsulation = 0;
> + SKB_GSO_CB(skb)->encap_level = 0;
> __skb_pull(skb, tnl_hlen);
> skb_reset_mac_header(skb);
> skb_set_network_header(skb, skb_inner_network_offset(skb));
>
^ permalink raw reply
* Re: [PATCH] net: Fix typos and whitespace.
From: Vlad Zolotarov @ 2016-03-23 17:16 UTC (permalink / raw)
To: Bjorn Helgaas, Bjorn Helgaas; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <20160323135535.GA18845@localhost>
On 03/23/2016 03:55 PM, Bjorn Helgaas wrote:
> [+cc Vlad]
>
> On Wed, Mar 23, 2016 at 08:45:30AM -0500, Bjorn Helgaas wrote:
>> Fix typos. Capitalize CPU, NAPI, RCU consistently. Align structure
>> indentation. No functional change intended; only comment and whitespace
>> changes.
>>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>> include/linux/netdevice.h | 215 ++++++++++++++++++++++-----------------------
>> @@ -920,7 +920,7 @@ struct tc_to_netdev {
>> *
>> * Enable or disable the VF ability to query its RSS Redirection Table and
>> * Hash Key. This is needed since on some devices VF share this information
>> - * with PF and querying it may adduce a theoretical security risk.
>> + * with PF and querying it may introduce a theoretical security risk.
>> * int (*ndo_set_vf_rss_query_en)(struct net_device *dev, int vf, bool setting)
> Most of these are obvious typo fixes, but I want to point out this one
> in particular because "adduce" is a real word. It's possible that's
> what Vlad intended in 01a3d796813d ("if_link: Add an additional
> parameter to ifla_vf_info for RSS querying"), though it doesn't quite
> seem to fit. Vlad, if you did intend "adduce", my apologies in
> advance.
Of course it was a typo :D
Thanks for fixing it, Bjorn.
>
> Bjorn
^ permalink raw reply
* Re: net/sctp: stack-out-of-bounds in sctp_getsockopt
From: Eric Dumazet @ 2016-03-23 17:14 UTC (permalink / raw)
To: Baozeng
Cc: pablo, kaber, kadlec, coreteam, netfilter-devel,
Vladislav Yasevich, nhorman, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CAP=GMUFA-qAOxF1W=aeWDYBiR-GSohJYGHiMDsrqwsD0xdg3_Q@mail.gmail.com>
On Thu, 2016-03-24 at 00:42 +0800, Baozeng wrote:
> Thanks for your quick patch. I tested it but it still reproduce the
> bug. We should limit the length of the name,
> not the prefix. The following patch fixs it.
>
> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> index 67b2e27..4837425 100644
> --- a/net/bridge/netfilter/ebtables.c
> +++ b/net/bridge/netfilter/ebtables.c
> @@ -346,7 +346,7 @@ find_inlist_lock(struct list_head *head, const
> char *name, const char *prefix,
> {
> return try_then_request_module(
> find_inlist_lock_noload(head, name, error, mutex),
> - "%s%s", prefix, name);
> + "%s%.*s", prefix, EBT_TABLE_MAXNAMELEN, name);
> }
>
Right you are, please send a formal patch ?
Thanks !
^ permalink raw reply
* Re: [RFC PATCH 7/9] GSO: Support partial segmentation offload
From: Tom Herbert @ 2016-03-23 17:09 UTC (permalink / raw)
To: Alexander Duyck
Cc: Edward Cree, Linux Kernel Network Developers, David S. Miller,
Alexander Duyck
In-Reply-To: <20160318232522.14955.13475.stgit@localhost.localdomain>
On Fri, Mar 18, 2016 at 4:25 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch adds support for something I am referring to as GSO partial.
> The basic idea is that we can support a broader range of devices for
> segmentation if we use fixed outer headers and have the hardware only
> really deal with segmenting the inner header. The idea behind the naming
> is due to the fact that everything before csum_start will be fixed headers,
> and everything after will be the region that is handled by hardware.
>
Personally, I don't like the name ;-) This technique should be
"generic" to handle almost all GSO except those case where headers are
not fixed which we should be able to avoid as a design point in any
new encapsulations. Besides, what if someday we perform GSO on
something where csum_start is not set?
Can you add some description about strategy for dealing with ip_id?
Thanks,
Tom
> With the current implementation it allows us to add support for the
> following GSO types with an inner TSO or TSO6 offload:
> NETIF_F_GSO_GRE
> NETIF_F_GSO_GRE_CSUM
> NETIF_F_UDP_TUNNEL
> NETIF_F_UDP_TUNNEL_CSUM
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> include/linux/netdev_features.h | 7 ++++++-
> include/linux/netdevice.h | 2 ++
> include/linux/skbuff.h | 7 ++++++-
> net/core/dev.c | 31 ++++++++++++++++++++++++++++++-
> net/core/ethtool.c | 1 +
> net/core/skbuff.c | 21 ++++++++++++++++++++-
> net/ipv4/af_inet.c | 12 ++++++++++--
> net/ipv4/gre_offload.c | 23 +++++++++++++++++++----
> net/ipv4/tcp_offload.c | 10 ++++++++--
> net/ipv4/udp_offload.c | 20 ++++++++++++++++----
> net/ipv6/ip6_offload.c | 9 ++++++++-
> 11 files changed, 126 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index a734bf43d190..8df3c5553af0 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -48,8 +48,12 @@ enum {
> NETIF_F_GSO_UDP_TUNNEL_BIT, /* ... UDP TUNNEL with TSO */
> NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO & CSUM */
> NETIF_F_GSO_TUNNEL_REMCSUM_BIT, /* ... TUNNEL with TSO & REMCSUM */
> + NETIF_F_GSO_PARTIAL_BIT, /* ... Only segment inner-most L4
> + * in hardware and all other
> + * headers in software.
> + */
> /**/NETIF_F_GSO_LAST = /* last bit, see GSO_MASK */
> - NETIF_F_GSO_TUNNEL_REMCSUM_BIT,
> + NETIF_F_GSO_PARTIAL_BIT,
>
> NETIF_F_FCOE_CRC_BIT, /* FCoE CRC32 */
> NETIF_F_SCTP_CRC_BIT, /* SCTP checksum offload */
> @@ -121,6 +125,7 @@ enum {
> #define NETIF_F_GSO_UDP_TUNNEL __NETIF_F(GSO_UDP_TUNNEL)
> #define NETIF_F_GSO_UDP_TUNNEL_CSUM __NETIF_F(GSO_UDP_TUNNEL_CSUM)
> #define NETIF_F_GSO_TUNNEL_REMCSUM __NETIF_F(GSO_TUNNEL_REMCSUM)
> +#define NETIF_F_GSO_PARTIAL __NETIF_F(GSO_PARTIAL)
> #define NETIF_F_HW_VLAN_STAG_FILTER __NETIF_F(HW_VLAN_STAG_FILTER)
> #define NETIF_F_HW_VLAN_STAG_RX __NETIF_F(HW_VLAN_STAG_RX)
> #define NETIF_F_HW_VLAN_STAG_TX __NETIF_F(HW_VLAN_STAG_TX)
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 31474d9d8a96..427d748ad8f9 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1647,6 +1647,7 @@ struct net_device {
> netdev_features_t vlan_features;
> netdev_features_t hw_enc_features;
> netdev_features_t mpls_features;
> + netdev_features_t gso_partial_features;
>
> int ifindex;
> int group;
> @@ -4014,6 +4015,7 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
> BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL != (NETIF_F_GSO_UDP_TUNNEL >> NETIF_F_GSO_SHIFT));
> BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL_CSUM != (NETIF_F_GSO_UDP_TUNNEL_CSUM >> NETIF_F_GSO_SHIFT));
> BUILD_BUG_ON(SKB_GSO_TUNNEL_REMCSUM != (NETIF_F_GSO_TUNNEL_REMCSUM >> NETIF_F_GSO_SHIFT));
> + BUILD_BUG_ON(SKB_GSO_PARTIAL != (NETIF_F_GSO_PARTIAL >> NETIF_F_GSO_SHIFT));
>
> return (features & feature) == feature;
> }
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 15d0df943466..c291a282f8b6 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -482,6 +482,8 @@ enum {
> SKB_GSO_UDP_TUNNEL_CSUM = 1 << 11,
>
> SKB_GSO_TUNNEL_REMCSUM = 1 << 12,
> +
> + SKB_GSO_PARTIAL = 1 << 13,
> };
>
> #if BITS_PER_LONG > 32
> @@ -3584,7 +3586,10 @@ static inline struct sec_path *skb_sec_path(struct sk_buff *skb)
> * Keeps track of level of encapsulation of network headers.
> */
> struct skb_gso_cb {
> - int mac_offset;
> + union {
> + int mac_offset;
> + int data_offset;
> + };
> int encap_level;
> __wsum csum;
> __u16 csum_start;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index edb7179bc051..666cf427898b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2711,6 +2711,19 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> return ERR_PTR(err);
> }
>
> + /* Only report GSO partial support if it will enable us to
> + * support segmentation on this frame without needing additional
> + * work.
> + */
> + if (features & NETIF_F_GSO_PARTIAL) {
> + netdev_features_t partial_features;
> + struct net_device *dev = skb->dev;
> +
> + partial_features = dev->features & dev->gso_partial_features;
> + if (!skb_gso_ok(skb, features | partial_features))
> + features &= ~NETIF_F_GSO_PARTIAL;
> + }
> +
> BUILD_BUG_ON(SKB_SGO_CB_OFFSET +
> sizeof(*SKB_GSO_CB(skb)) > sizeof(skb->cb));
>
> @@ -2841,6 +2854,14 @@ netdev_features_t netif_skb_features(struct sk_buff *skb)
> if (skb->encapsulation)
> features &= dev->hw_enc_features;
>
> + /* Support for GSO partial features requires software intervention
> + * before we can actually process the packets so we need to strip
> + * support for any partial features now and we can pull them back
> + * in after we have partially segmented the frame.
> + */
> + if (skb_is_gso(skb) && !(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL))
> + features &= ~dev->gso_partial_features;
> +
> if (skb_vlan_tagged(skb))
> features = netdev_intersect_features(features,
> dev->vlan_features |
> @@ -6702,6 +6723,14 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
> }
> }
>
> + /* GSO partial features require GSO partial be set */
> + if ((features & dev->gso_partial_features) &&
> + !(features & NETIF_F_GSO_PARTIAL)) {
> + netdev_dbg(dev,
> + "Dropping partially supported GSO features since no GSO partial.\n");
> + features &= ~dev->gso_partial_features;
> + }
> +
> #ifdef CONFIG_NET_RX_BUSY_POLL
> if (dev->netdev_ops->ndo_busy_poll)
> features |= NETIF_F_BUSY_POLL;
> @@ -6982,7 +7011,7 @@ int register_netdevice(struct net_device *dev)
>
> /* Make NETIF_F_SG inheritable to tunnel devices.
> */
> - dev->hw_enc_features |= NETIF_F_SG;
> + dev->hw_enc_features |= NETIF_F_SG | NETIF_F_GSO_PARTIAL;
>
> /* Make NETIF_F_SG inheritable to MPLS.
> */
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index b3c39d531469..d1b278c6c29f 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -88,6 +88,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
> [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
> [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
> [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-remcsum-segmentation",
> + [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
>
> [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc",
> [NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp",
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index f044f970f1a6..bdcba77e164c 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3076,8 +3076,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
> struct sk_buff *frag_skb = head_skb;
> unsigned int offset = doffset;
> unsigned int tnl_hlen = skb_tnl_header_len(head_skb);
> + unsigned int partial_segs = 0;
> unsigned int headroom;
> - unsigned int len;
> + unsigned int len = head_skb->len;
> __be16 proto;
> bool csum;
> int sg = !!(features & NETIF_F_SG);
> @@ -3094,6 +3095,15 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> csum = !!can_checksum_protocol(features, proto);
>
> + /* GSO partial only requires that we trim off any excess that
> + * doesn't fit into an MSS sized block, so take care of that
> + * now.
> + */
> + if (features & NETIF_F_GSO_PARTIAL) {
> + partial_segs = len / mss;
> + mss *= partial_segs;
> + }
> +
> headroom = skb_headroom(head_skb);
> pos = skb_headlen(head_skb);
>
> @@ -3281,6 +3291,15 @@ perform_csum_check:
> */
> segs->prev = tail;
>
> + /* Update GSO info on first skb in partial sequence. */
> + if (partial_segs) {
> + skb_shinfo(segs)->gso_size = mss / partial_segs;
> + skb_shinfo(segs)->gso_segs = partial_segs;
> + skb_shinfo(segs)->gso_type = skb_shinfo(head_skb)->gso_type |
> + SKB_GSO_PARTIAL;
> + SKB_GSO_CB(segs)->data_offset = skb_headroom(segs) + doffset;
> + }
> +
> /* Following permits correct backpressure, for protocols
> * using skb_set_owner_w().
> * Idea is to tranfert ownership from head_skb to last segment.
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 5e3885672907..d091f91fa25d 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1199,7 +1199,7 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> unsigned int offset = 0;
> bool udpfrag, encap;
> struct iphdr *iph;
> - int proto;
> + int proto, tot_len;
> int nhoff;
> int ihl;
> int id;
> @@ -1269,10 +1269,18 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> if (skb->next)
> iph->frag_off |= htons(IP_MF);
> offset += skb->len - nhoff - ihl;
> + tot_len = skb->len - nhoff;
> + } else if (skb_is_gso(skb)) {
> + iph->id = htons(id);
> + id += skb_shinfo(skb)->gso_segs;
> + tot_len = skb_shinfo(skb)->gso_size +
> + SKB_GSO_CB(skb)->data_offset +
> + skb->head - (unsigned char *)iph;
> } else {
> iph->id = htons(id++);
> + tot_len = skb->len - nhoff;
> }
> - iph->tot_len = htons(skb->len - nhoff);
> + iph->tot_len = htons(tot_len);
> ip_send_check(iph);
> if (encap)
> skb_reset_inner_headers(skb);
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index 7ea14ced9222..dea0390d65bb 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -85,7 +85,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> skb = segs;
> do {
> struct gre_base_hdr *greh;
> - __be32 *pcsum;
> + __sum16 *pcsum;
>
> /* Set up inner headers if we are offloading inner checksum */
> if (skb->ip_summed == CHECKSUM_PARTIAL) {
> @@ -105,10 +105,25 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> continue;
>
> greh = (struct gre_base_hdr *)skb_transport_header(skb);
> - pcsum = (__be32 *)(greh + 1);
> + pcsum = (__sum16 *)(greh + 1);
> +
> + if (skb_is_gso(skb)) {
> + unsigned int partial_adj;
> +
> + /* Adjust checksum to account for the fact that
> + * the partial checksum is based on actual size
> + * whereas headers should be based on MSS size.
> + */
> + partial_adj = skb->len + skb_headroom(skb) -
> + SKB_GSO_CB(skb)->data_offset -
> + skb_shinfo(skb)->gso_size;
> + *pcsum = ~csum_fold((__force __wsum)htonl(partial_adj));
> + } else {
> + *pcsum = 0;
> + }
>
> - *pcsum = 0;
> - *(__sum16 *)pcsum = gso_make_checksum(skb, 0);
> + *(pcsum + 1) = 0;
> + *pcsum = gso_make_checksum(skb, 0);
> } while ((skb = skb->next));
> out:
> return segs;
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index 1a2e9957c177..4e9b8f011473 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -107,6 +107,12 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
> goto out;
> }
>
> + /* GSO partial only requires splitting the frame into an MSS
> + * multiple and possibly a remainder. So update the mss now.
> + */
> + if (features & NETIF_F_GSO_PARTIAL)
> + mss = skb->len - (skb->len % mss);
> +
> copy_destructor = gso_skb->destructor == tcp_wfree;
> ooo_okay = gso_skb->ooo_okay;
> /* All segments but the first should have ooo_okay cleared */
> @@ -131,7 +137,7 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
> newcheck = ~csum_fold((__force __wsum)((__force u32)th->check +
> (__force u32)delta));
>
> - do {
> + while (skb->next) {
> th->fin = th->psh = 0;
> th->check = newcheck;
>
> @@ -151,7 +157,7 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
>
> th->seq = htonl(seq);
> th->cwr = 0;
> - } while (skb->next);
> + }
>
> /* Following permits TCP Small Queues to work well with GSO :
> * The callback to TCP stack will be called at the time last frag
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 8a3405a80260..5fcb93269afb 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -100,7 +100,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> udp_offset = outer_hlen - tnl_hlen;
> skb = segs;
> do {
> - __be16 len;
> + unsigned int len;
>
> if (remcsum)
> skb->ip_summed = CHECKSUM_NONE;
> @@ -118,14 +118,26 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> skb_reset_mac_header(skb);
> skb_set_network_header(skb, mac_len);
> skb_set_transport_header(skb, udp_offset);
> - len = htons(skb->len - udp_offset);
> + len = skb->len - udp_offset;
> uh = udp_hdr(skb);
> - uh->len = len;
> +
> + /* If we are only performing partial GSO the inner header
> + * will be using a length value equal to only one MSS sized
> + * segment instead of the entire frame.
> + */
> + if (skb_is_gso(skb)) {
> + uh->len = htons(skb_shinfo(skb)->gso_size +
> + SKB_GSO_CB(skb)->data_offset +
> + skb->head - (unsigned char *)uh);
> + } else {
> + uh->len = htons(len);
> + }
>
> if (!need_csum)
> continue;
>
> - uh->check = ~csum_fold(csum_add(partial, (__force __wsum)len));
> + uh->check = ~csum_fold(csum_add(partial,
> + (__force __wsum)htonl(len)));
>
> if (skb->encapsulation || !offload_csum) {
> uh->check = gso_make_checksum(skb, ~uh->check);
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index eeca943f12dc..d467053c226a 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -63,6 +63,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
> int proto;
> struct frag_hdr *fptr;
> unsigned int unfrag_ip6hlen;
> + unsigned int payload_len;
> u8 *prevhdr;
> int offset = 0;
> bool encap, udpfrag;
> @@ -117,7 +118,13 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>
> for (skb = segs; skb; skb = skb->next) {
> ipv6h = (struct ipv6hdr *)(skb_mac_header(skb) + nhoff);
> - ipv6h->payload_len = htons(skb->len - nhoff - sizeof(*ipv6h));
> + if (skb_is_gso(skb))
> + payload_len = skb_shinfo(skb)->gso_size +
> + SKB_GSO_CB(skb)->data_offset +
> + skb->head - (unsigned char *)(ipv6h + 1);
> + else
> + payload_len = skb->len - nhoff - sizeof(*ipv6h);
> + ipv6h->payload_len = htons(payload_len);
> skb->network_header = (u8 *)ipv6h - skb->head;
>
> if (udpfrag) {
>
^ permalink raw reply
* Re: [PATCH] net: mlxsw: avoid unused variable warnings
From: Andrew Lunn @ 2016-03-23 16:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jiri Pirko, Ido Schimmel, David S. Miller, Elad Raz, netdev,
linux-kernel
In-Reply-To: <1458751080-2044921-1-git-send-email-arnd@arndb.de>
On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
> not set, which can lead to unused variable warnings:
>
> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
>
> This changes the mlxsw driver to remove the local variables we get
> warnings for and instead pass the device directly into the API.
Hi Arnd
Would it not be better to fix the macro?
I think the issue is that dev_dbg_ratelimited calls no_printk(),
without making use of dev. So how about:
#define dev_dbg_ratelimited(dev, fmt, ...) \
({ \
if (0) \
dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
})
This follows the pattern for other macros for when DEBUG is not defined.
Andrew
^ 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