* Re: [PATCH net-next v3.16]r8169: Correct value from speed 10 on MII_BMCR
From: Corcodel Marian @ 2016-03-25 13:35 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev, Francois Romieu
In-Reply-To: <20160325133113.3001B62813@mail.nwl.cc>
On Fri, 25 Mar 2016 14:33:02 +0100
Phil Sutter <phil@nwl.cc> wrote:
> Hi,
>
> Your patch submissions are getting better, also good to see you're
> finally using git-send-email. A few things need to be corrected
> though:
>
> Subject line:
> - The 'vNN' part in brackets is supposed to be the reroll-count (see
> 'git format-patch -h' for details), not kernel version. You're
> supposed to submit your patches for either net or net-next, not a
> specific kernel version like 3.16.
> - Missing space after the closing bracket.
> - Extra space after 'r8169:'.
>
> On Fri, Mar 25, 2016 at 03:01:06PM +0200, Corcodel Marian wrote:
> > This patch correct value on MII_BMCR register ald value 0
> > have target on reserved register first 2 bytes from MII_BMCR
> > speed 10 is flipped value on BMCR_SPEED100
>
> This is very hard to understand. I *guess* you want to say one should
> not write 0 to MII_BMCR since it overwrites reserved bits, but it's
> really not clear. Don't you know someone who can properly translate
> from Romanian to English?
>
> Also detailed instructions on how to trigger the problem you are
> fixing for would be good. In detail: Which specific hardware was
> used, in which situation did the problem occur, how did it behave in
> that situation and what was the expected behaviour?
>
> Cheers, Phil
> This is very hard to understand. I *guess* you want to say one should
> not write 0 to MII_BMCR since it overwrites reserved bits, but it's
> really not clear. Don't you know someone who can properly translate
> from Romanian to English?
>Yes this it.
^ permalink raw reply
* Re: [PATCH net-next v3.16]r8169: Implement definitions from linux/mii.h header
From: Corcodel Marian @ 2016-03-25 14:00 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, Francois Romieu
In-Reply-To: <56F524FB.5080202@cogentembedded.com>
On Fri, 25 Mar 2016 14:46:03 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 3/25/2016 1:25 PM, Corcodel Marian wrote:
>
> > Add definitions from mii and inhibit 0 to advertise.
> >
> > Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
> > ---
> > drivers/net/ethernet/realtek/r8169.c | 22 ++++++----------------
> > 1 file changed, 6 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/r8169.c
> > b/drivers/net/ethernet/realtek/r8169.c index a57b650..a450656 100644
> > --- a/drivers/net/ethernet/realtek/r8169.c
> > +++ b/drivers/net/ethernet/realtek/r8169.c
> > @@ -1679,29 +1679,19 @@ static int rtl8169_set_speed_xmii(struct
> > net_device *dev, int auto_nego;
> >
> > auto_nego = rtl_readphy(tp, MII_ADVERTISE);
> > - auto_nego &= ~(ADVERTISE_10HALF | ADVERTISE_10FULL
> > |
> > - ADVERTISE_100HALF |
> > ADVERTISE_100FULL); -
> > - if (adv & ADVERTISED_10baseT_Half)
> > - auto_nego |= ADVERTISE_10HALF;
> > - if (adv & ADVERTISED_10baseT_Full)
> > - auto_nego |= ADVERTISE_10FULL;
> > - if (adv & ADVERTISED_100baseT_Half)
> > - auto_nego |= ADVERTISE_100HALF;
> > - if (adv & ADVERTISED_100baseT_Full)
> > - auto_nego |= ADVERTISE_100FULL;
> > + auto_nego &= ~(0 | ADVERTISE_10HALF |
> > ADVERTISE_10FULL |
>
> ORing with 0 doesn't change the result.
>
> > + ADVERTISE_100HALF |
> > ADVERTISE_100FULL |
> > + ADVERTISE_PAUSE_CAP |
> > ADVERTISE_PAUSE_ASYM);
> >
> > + auto_nego |= ethtool_adv_to_mii_adv_t(adv);
> > auto_nego |= ADVERTISE_PAUSE_CAP |
> > ADVERTISE_PAUSE_ASYM;
> >
> > giga_ctrl = rtl_readphy(tp, MII_CTRL1000);
> > - giga_ctrl &= ~(ADVERTISE_1000FULL |
> > ADVERTISE_1000HALF);
> > + giga_ctrl &= ~(0 | ADVERTISE_1000FULL |
> > ADVERTISE_1000HALF);
>
> Likewise.
>
> [...]
>
> MBR, Sergei
>
>Zero not change result but is defined mii_adv_to_ethtool_adv_t,
>ethtool_adv_to_mii_ctrl1000_t and not want to set because on
>first bit from register is set on default and only
>CSMA/CD protocol is supported.For more info see mii.h file from
>kernel
^ permalink raw reply
* [PATCH net-next v3.16]r8169: Add support for half duplex on MDI
From: Corcodel Marian @ 2016-03-25 14:29 UTC (permalink / raw)
To: netdev; +Cc: Francois Romieu, Corcodel Marian
This patch add support for half duplex on MDI chips.
Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
---
drivers/net/ethernet/realtek/r8169.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 77c5efb..7f6fb1f 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1713,8 +1713,13 @@ static int rtl8169_set_speed_xmii(struct net_device *dev,
else
goto out;
- if (duplex == DUPLEX_FULL)
+ if (duplex == DUPLEX_FULL) {
bmcr |= BMCR_FULLDPLX;
+ } else if (duplex == DUPLEX_HALF) {
+ bmcr |= ~BMCR_FULLDPLX;
+ } else {
+ goto out;
+ }
}
rtl_writephy(tp, MII_BMCR, bmcr);
--
2.1.4
^ permalink raw reply related
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 14:35 UTC (permalink / raw)
To: Vijay Pandurangan; +Cc: Cong Wang, netdev, Evan Jones, Cong Wang
In-Reply-To: <CAKUBDd9qs9rzrvp8XJ4HpddekMsRjmL+rwsL3DqTWutPon=NdA@mail.gmail.com>
On 03/24/2016 10:24 PM, Vijay Pandurangan wrote:
> On Fri, Mar 25, 2016 at 1:07 AM, Ben Greear <greearb@candelatech.com> wrote:
>> On 03/24/2016 09:45 PM, Vijay Pandurangan wrote:
>>>
>>> Actually, maybe they should be set to CHECKSUM_PARTIAL if we want veth
>>> to drop the packets if they have bad checksums before they hit the
>>> application level.
>>
>>
>> VETH is pretty special in that when you transmit a frame on one
>> device, it's pair receives it, and unless there is RAM corruption
>> or bugs in the kernel, then it cannot be corrupted.
>
> Yeah, you're right that that's an optimization. However, I think that
> we should first ensure that
>
> a->veth->b
>
> operates exactly like:
>
> a->physical eth 1 -> physical eth 2->b
>
> in all cases. Once we have that working everywhere we could think
> about optimizations.
>
>
> If we're willing to refactor, we could implement the optimization by
> allowing veth devices to know whether their immediate peer is. If a
> veth knows it's talking to another veth, it could under some
> circumstances elide checksum calculation and verification. I'm not
> sure what abstractions that would break, though. What do you guys
> think?
veth ALWAYS transmits to another VETH. The problem is that when veth is
given a packet to transmit, it is difficult to know where that packet
came from.
And, adding software checksumming to veth for every frame would be a huge
performance hit.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 00/13] Enhance stmmac driver to support GMAC4.x IP
From: David Miller @ 2016-03-25 15:11 UTC (permalink / raw)
To: alexandre.torgue; +Cc: netdev, peppe.cavallaro, lars.persson
In-Reply-To: <1458897260-14104-1-git-send-email-alexandre.torgue@st.com>
It is absolutely not appropriate to submit new feature patches
at this time.
Please resubmit this after the net-next tree opens back out.
Thank you.
^ permalink raw reply
* Re: [PATCH 00/13] Enhance stmmac driver to support GMAC4.x IP
From: Alexandre Torgue @ 2016-03-25 15:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, peppe.cavallaro, lars.persson
In-Reply-To: <20160325.111153.1753782955238285726.davem@davemloft.net>
Hi,
On 03/25/2016 04:11 PM, David Miller wrote:
>
> It is absolutely not appropriate to submit new feature patches
> at this time.
>
> Please resubmit this after the net-next tree opens back out.
>
No pb, I will wait and resend.
Regards
Alex
> Thank you.
>
^ permalink raw reply
* Re: [PATCH v2] netpoll: Fix extra refcount release in netpoll_cleanup()
From: David Miller @ 2016-03-25 15:24 UTC (permalink / raw)
To: bhelgaas; +Cc: nikolay, netdev, nhorman, aduyck, linux-kernel
In-Reply-To: <20160325025621.32476.93348.stgit@bhelgaas-glaptop2.roam.corp.google.com>
From: Bjorn Helgaas <bhelgaas@google.com>
Date: Thu, 24 Mar 2016 21:56:21 -0500
> netpoll_setup() does a dev_hold() on np->dev, the netpoll device. If it
> fails, it correctly does a dev_put() but leaves np->dev set. If we call
> netpoll_cleanup() after the failure, np->dev is still set so we do another
> dev_put(), which decrements the refcount an extra time.
>
> It's questionable to call netpoll_cleanup() after netpoll_setup() fails,
> but it can be difficult to find the problem, and we can easily avoid it in
> this case. The extra decrements can lead to hangs like this:
>
> unregister_netdevice: waiting for bond0 to become free. Usage count = -3
>
> Set and clear np->dev at the points where we dev_hold() and dev_put() the
> device.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Looks good, applied and queued up for -stable.
But you probably do want to look into the stacked device issue
Neil pointed out.
Thanks!
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Craig Gallek @ 2016-03-25 15:29 UTC (permalink / raw)
To: Linux Kernel Network Developers
On Thu, Mar 24, 2016 at 2:00 PM, Willy Tarreau <w@1wt.eu> wrote:
> The pattern is :
>
> t0 : unprivileged processes 1 and 2 are listening to the same port
> (sock1@pid1) (sock2@pid2)
> <------ listening ------>
>
> t1 : new processes are started to replace the old ones
> (sock1@pid1) (sock2@pid2) (sock3@pid3) (sock4@pid4)
> <------ listening ------> <------ listening ------>
>
> t2 : new processes signal the old ones they must stop
> (sock1@pid1) (sock2@pid2) (sock3@pid3) (sock4@pid4)
> <------- draining ------> <------ listening ------>
>
> t3 : pids 1 and 2 have finished, they go away
> (sock3@pid3) (sock4@pid4)
> <------ gone -----> <------ listening ------>
To address the documentation issues, I'd like to reference the following:
- The filter.txt document in the kernel tree:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/filter.txt
- It uses (and extends) the BPF instruction set defined in the
original BSD BPF paper: http://www.tcpdump.org/papers/bpf-usenix93.pdf
- The kernel headers define all of the user-space structures used:
* https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/filter.h
* https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/bpf.h
I've been trying to come up with an example BPF program for use in the
example Willy gave earlier in this thread (using 4 points in time and
describing one process with two listening sockets replacing another
with two listening sockets). Everything except the last step is
pretty straight forward using what is currently available in the
kernel. I'm using random distribution for simplicity, but you could
easily do something smarter using more information about the specific
hardware:
t0: Evenly distrubute load to two SO_REUSEPORT sockets in a single process:
ld rand
mod #2
ret a
t1: Fork a new process, create two new listening sockets in the same
group. Even after calling listen(), but before updating the BPF
program, only the first two sockets will see new connections. The
program is trivially modified to use all 4.
ld rand
mod #4
ret a
t2: Stop sending new connections to the first two sockets (draining)
ld rand
mod #2
add #2
ret a
t3: Close the first two sockets and only use the last two. This is
the tricky step. Before this point, the sockets are numbered 0
through 3 from the perspective of the BPF program (in the order
listen() was called). As soon as socket 0 is closed, the last socket
in the list replaces it (what was 3 becomes 0). When socket 1 is
closed, socket 2 moves into that position. The assumptions about the
socket indexes in the BPF program need to change as the indexes change
as a result of closing them.
Even if you use an EBPF map as a level of indirection here, you still
have the issue that the socket indexes change as a result of some of
them leaving the group. I'm not sure yet how to properly fix this,
but it will probably mean changing the way the socket indexing
works... The current scheme is really an implementation detail
optimized for efficiency. It may be worth modifying or creating a
mode which results in a stable mapping. This will probably be
necessary for any scheme which expects sockets to regularly enter or
leave the group.
^ permalink raw reply
* Re: dynamically set number of queues for 82598 devices in -stable
From: William Dauchy @ 2016-03-25 15:22 UTC (permalink / raw)
To: Jeff Kirsher
Cc: William Dauchy, intel-wired-lan, netdev, Emil Tantilov,
David S. Miller
In-Reply-To: <20151127100557.GB21147@gandi.net>
[-- Attachment #1: Type: text/plain, Size: 524 bytes --]
Hi Jeff,
Any chance to get an opinion on that?
Thanks,
On Nov27 11:05, William Dauchy wrote:
> Hello Jeff,
>
> I faced the problem described in commit 7e3f5c8:
> ixgbe: fix bounds checking in ixgbe_setup_tc for 82598
>
> This patch resolves an issue where users were not able to dynamically
> set number of queues for 82598 via ethtool -L
>
>
> I backported it for my v4.1.x build but I was wondering if this could be
> a candidate for a backport in stable tree at least for v4.1.x.
--
William
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH net] bpf: add missing map_flags to bpf_map_show_fdinfo
From: David Miller @ 2016-03-25 15:37 UTC (permalink / raw)
To: daniel; +Cc: alexei.starovoitov, netdev
In-Reply-To: <5da67066a9739eb443b2e89e58ea4d356d70459c.1458861762.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 25 Mar 2016 00:30:25 +0100
> Add map_flags attribute to bpf_map_show_fdinfo(), so that tools like
> tc can check for them when loading objects from a pinned entry, e.g.
> if user intent wrt allocation (BPF_F_NO_PREALLOC) is different to the
> pinned object, it can bail out. Follow-up to 6c9059817432 ("bpf:
> pre-allocate hash map elements"), so that tc can still support this
> with v4.6.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [PATCH] net: phy: bcm7xxx: Add entries for Broadcom BCM7346 and BCM7362
From: David Miller @ 2016-03-25 15:38 UTC (permalink / raw)
To: jaedon.shin; +Cc: netdev, f.fainelli
In-Reply-To: <1458877614-94202-1-git-send-email-jaedon.shin@gmail.com>
From: Jaedon Shin <jaedon.shin@gmail.com>
Date: Fri, 25 Mar 2016 12:46:54 +0900
> Add PHY entries for the Broadcom BCM7346 and BCM7362 chips, these are
> 40nm generation Ethernet PHY.
>
> Fixes: 815717d1473e ("net: phy: bcm7xxx: Remove wildcard entries")
> Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next 1/1] qlge: Update version to 1.00.00.35
From: David Miller @ 2016-03-25 15:41 UTC (permalink / raw)
To: manish.chopra; +Cc: netdev, Dept-GELinuxNICDev, harish.patil
In-Reply-To: <1458904449-5776-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@qlogic.com>
Date: Fri, 25 Mar 2016 07:14:09 -0400
> Just updating version as many fixes got
> accumulated over 1.00.00.34
>
> Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
Applied, but it is the 'net' tree that fixes et al. like this
should be targetting as 'net-next' is closed.
^ permalink raw reply
* Re: [PATCH v2 1/1] net: macb: remove BUG_ON() and reset the queue to handle RX errors
From: David Miller @ 2016-03-25 15:44 UTC (permalink / raw)
To: cyrille.pitchen
Cc: nicolas.ferre, linux-arm-kernel, netdev, soren.brinkmann,
linux-kernel, narmstrong
In-Reply-To: <1458898654-25864-1-git-send-email-cyrille.pitchen@atmel.com>
From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Date: Fri, 25 Mar 2016 10:37:34 +0100
> This patch removes two BUG_ON() used to notify about RX queue corruptions
> on macb (not gem) hardware without actually handling the error.
>
> The new code skips corrupted frames but still processes faultless frames.
> Then it resets the RX queue before restarting the reception from a clean
> state.
>
> This patch is a rework of an older patch proposed by Neil Armstrong:
> http://patchwork.ozlabs.org/patch/371525/
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v3.16]r8169: Add support for half duplex on MDI
From: Andrew Lunn @ 2016-03-25 15:50 UTC (permalink / raw)
To: Corcodel Marian; +Cc: netdev, Francois Romieu
In-Reply-To: <1458916193-32521-1-git-send-email-asd@marian1000.go.ro>
On Fri, Mar 25, 2016 at 04:29:52PM +0200, Corcodel Marian wrote:
> This patch add support for half duplex on MDI chips.
>
> Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
> ---
> drivers/net/ethernet/realtek/r8169.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 77c5efb..7f6fb1f 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -1713,8 +1713,13 @@ static int rtl8169_set_speed_xmii(struct net_device *dev,
> else
> goto out;
>
> - if (duplex == DUPLEX_FULL)
> + if (duplex == DUPLEX_FULL) {
> bmcr |= BMCR_FULLDPLX;
> + } else if (duplex == DUPLEX_HALF) {
> + bmcr |= ~BMCR_FULLDPLX;
> + } else {
> + goto out;
> + }
Hi Corcodel
What PHY is actually being used? One that is supported by
drivers/net/phy/realtek.c?
It looks like the bugs you are finding are all handling stuff which is
generic, and well tested code in drivers/net/phy/phy_device.c would do
for you, with less bugs.
Maybe you want to consider throwing away the PHY code in r8169 and
just implement an standard mdio bus, use phylib and the realtek.c phy
driver?
Andrew
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Ben Greear @ 2016-03-25 16:10 UTC (permalink / raw)
To: Cong Wang; +Cc: Vijay Pandurangan, netdev, Evan Jones, Cong Wang
In-Reply-To: <CAM_iQpWwch3+6u+X0DBGyVSWttd7Nz2COdmq2q=G9BxisW52bA@mail.gmail.com>
On 03/24/2016 10:33 PM, Cong Wang wrote:
> On Thu, Mar 24, 2016 at 10:13 PM, Ben Greear <greearb@candelatech.com> wrote:
>>
>>
>> On 03/24/2016 10:06 PM, Cong Wang wrote:
>>>
>>> On Thu, Mar 24, 2016 at 9:34 PM, Ben Greear <greearb@candelatech.com>
>>> wrote:
>>>>
>>>>
>>>>
>>>> On 03/24/2016 06:44 PM, Vijay Pandurangan wrote:
>>>>>
>>>>>
>>>>> Oops, I think my last email didn't go through due to an inadvertent
>>>>> html attachment from my phone mail client.
>>>>>
>>>>> Can you send us a copy of a packet you're sending and/or confirm that
>>>>> the IP and UDP4 checksums are set correctly in the packet?
>>>>>
>>>>> If those are set right, I think we need to read through the networking
>>>>> code again to see why this is broken...
>>>>
>>>>
>>>>
>>>> Wireshark decodes the packet as having no checksum errors.
>>>>
>>>> I think the contents of the packet is correct, but the 'ip_summed'
>>>> field is set incorrectly to 'NONE' when transmitting on a raw packet
>>>> socket.
>>>
>>>
>>> Yeah, these bugs are all due to the different interpretations of
>>> ip_summed on TX path and RX path. I think the following patch
>>> should work, if the comments don't mislead me. Could you give
>>> it a try?
>>>
>>> For the long term, we need to unify the meaning of ip_summed
>>> on TX path and RX path, or at least translate it in skb_scrub_packet().
>>
>>
>> I can test this tomorrow, but I think it will not work. I'm not sending raw
>> IP frames, I'm sending full ethernet frames. Socket is PF_PACKET, SOCK_RAW.
>>
>> Your patch may still be useful for others though?
>
> Here we go:
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 1ecfa71..ab66080 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1925,6 +1925,7 @@ static int packet_sendmsg_spkt(struct socket
> *sock, struct msghdr *msg,
> goto out_unlock;
> }
>
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
> skb->protocol = proto;
> skb->dev = dev;
> skb->priority = sk->sk_priority;
> @@ -2496,6 +2497,7 @@ static int tpacket_fill_skb(struct packet_sock
> *po, struct sk_buff *skb,
>
> ph.raw = frame;
>
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
> skb->protocol = proto;
> skb->dev = dev;
> skb->priority = po->sk.sk_priority;
> @@ -2805,6 +2807,7 @@ static struct sk_buff *packet_alloc_skb(struct
> sock *sk, size_t prepad,
> skb_put(skb, linear);
> skb->data_len = len - linear;
> skb->len += len - linear;
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> return skb;
> }
I am suspicious that this will break at least some drivers. I grepped around
for ip_summed, and found this, for instance:
davicom/dm9000.c
/* The DM9000 is not smart enough to leave fragmented packets alone. */
if (dm->ip_summed != ip_summed) {
if (ip_summed == CHECKSUM_NONE)
iow(dm, DM9000_TCCR, 0);
else
iow(dm, DM9000_TCCR, TCCR_IP | TCCR_UDP | TCCR_TCP);
dm->ip_summed = ip_summed;
}
It is taking action based on ip_summed == CHECKSUM_NONE, and your change
will probably break that.
I would suggest that we try to make any fix specific only to veth,
at least for now. A tree-wide audit of drivers is probably required
to safely make the kind of change you propose above.
So, unless you can explain why your change is safe, then I do not plan
to test it.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [ethtool PATCH v7 0/2] add support for new ETHTOOL_xLINKSETTINGS ioctls
From: David Decotigny @ 2016-03-25 16:21 UTC (permalink / raw)
To: netdev
Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati,
Joe Perches, David Decotigny
From: David Decotigny <decot@googlers.com>
History:
v7
added ref to related kernel commit in netlink ioctl patch description
v6
re-added last patch, to use AF_NETLINK when AF_INET not available
v5
rebased main patch, removed last patch "use AF_LOCAL when AF_INET
not available"
v4
review Ben Hutchings:
using AF_UNIX instead of INET6 in the absence of v4 sockets
use stdbool.h
do_seeprom always fails when offset/length out of bounds
sync to latest ethtool.h + kernel.h from net-next
__SANE_USERSPACE_TYPES__ always defined
cosmetic updates for var == const tests
cosmetic updates for associativity in tests
v3
TRUE/FALSE obvious-ification
v2
added do_seeprom patch
added netdev <at> as recipient
v1
initial submission
############################################
# Patch Set Summary:
David Decotigny (2):
ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls
ethtool: use netlink socket when AF_INET not available
configure.ac | 2 +-
ethtool.c | 688 ++++++++++++++++++++++++++++++++++++++++++++-------------
internal.h | 67 ++++++
test-cmdline.c | 13 ++
4 files changed, 611 insertions(+), 159 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* [ethtool PATCH v7 1/2] ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls
From: David Decotigny @ 2016-03-25 16:21 UTC (permalink / raw)
To: netdev
Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati,
Joe Perches, David Decotigny
In-Reply-To: <1458922862-98024-1-git-send-email-ddecotig@gmail.com>
From: David Decotigny <decot@googlers.com>
More info with kernel commit 8d3f2806f8fb ("Merge branch
'ethtool-ksettings'").
Note: The new features implemented in this patch depend on kernel
commit 793cf87de9d1 ("Set cmd field in ETHTOOL_GLINKSETTINGS response to
wrong nwords").
Signed-off-by: David Decotigny <decot@googlers.com>
---
ethtool.c | 681 ++++++++++++++++++++++++++++++++++++++++++++-------------
internal.h | 67 ++++++
test-cmdline.c | 13 ++
3 files changed, 603 insertions(+), 158 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 0cd0d4f..cb3d971 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -47,42 +47,6 @@
#define MAX_ADDR_LEN 32
#endif
-#define ALL_ADVERTISED_MODES \
- (ADVERTISED_10baseT_Half | \
- ADVERTISED_10baseT_Full | \
- ADVERTISED_100baseT_Half | \
- ADVERTISED_100baseT_Full | \
- ADVERTISED_1000baseT_Half | \
- ADVERTISED_1000baseT_Full | \
- ADVERTISED_1000baseKX_Full| \
- ADVERTISED_2500baseX_Full | \
- ADVERTISED_10000baseT_Full | \
- ADVERTISED_10000baseKX4_Full | \
- ADVERTISED_10000baseKR_Full | \
- ADVERTISED_10000baseR_FEC | \
- ADVERTISED_20000baseMLD2_Full | \
- ADVERTISED_20000baseKR2_Full | \
- ADVERTISED_40000baseKR4_Full | \
- ADVERTISED_40000baseCR4_Full | \
- ADVERTISED_40000baseSR4_Full | \
- ADVERTISED_40000baseLR4_Full | \
- ADVERTISED_56000baseKR4_Full | \
- ADVERTISED_56000baseCR4_Full | \
- ADVERTISED_56000baseSR4_Full | \
- ADVERTISED_56000baseLR4_Full)
-
-#define ALL_ADVERTISED_FLAGS \
- (ADVERTISED_Autoneg | \
- ADVERTISED_TP | \
- ADVERTISED_AUI | \
- ADVERTISED_MII | \
- ADVERTISED_FIBRE | \
- ADVERTISED_BNC | \
- ADVERTISED_Pause | \
- ADVERTISED_Asym_Pause | \
- ADVERTISED_Backplane | \
- ALL_ADVERTISED_MODES)
-
#ifndef HAVE_NETIF_MSG
enum {
NETIF_MSG_DRV = 0x0001,
@@ -293,6 +257,43 @@ static void get_mac_addr(char *src, unsigned char *dest)
}
}
+static int parse_hex_u32_bitmap(const char *s,
+ unsigned int nbits, u32 *result)
+{
+ const unsigned int nwords = __KERNEL_DIV_ROUND_UP(nbits, 32);
+ size_t slen = strlen(s);
+ size_t i;
+
+ /* ignore optional '0x' prefix */
+ if ((slen > 2) && (strncasecmp(s, "0x", 2) == 0)) {
+ slen -= 2;
+ s += 2;
+ }
+
+ if (slen > 8 * nwords) /* up to 2 digits per byte */
+ return -1;
+
+ memset(result, 0, 4 * nwords);
+ for (i = 0 ; i < slen ; ++i) {
+ const unsigned int shift = (slen - 1 - i) * 4;
+ u32 *dest = &result[shift / 32];
+ u32 nibble;
+
+ if ('a' <= s[i] && s[i] <= 'f')
+ nibble = 0xa + (s[i] - 'a');
+ else if ('A' <= s[i] && s[i] <= 'F')
+ nibble = 0xa + (s[i] - 'A');
+ else if ('0' <= s[i] && s[i] <= '9')
+ nibble = (s[i] - '0');
+ else
+ return -1;
+
+ *dest |= (nibble << (shift % 32));
+ }
+
+ return 0;
+}
+
static void parse_generic_cmdline(struct cmd_context *ctx,
int *changed,
struct cmdline_info *info,
@@ -472,64 +473,157 @@ static int do_version(struct cmd_context *ctx)
return 0;
}
-static void dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
- int link_mode_only);
+/* link mode routines */
-static void dump_supported(struct ethtool_cmd *ep)
+static __ETHTOOL_DECLARE_LINK_MODE_MASK(all_advertised_modes);
+static __ETHTOOL_DECLARE_LINK_MODE_MASK(all_advertised_flags);
+
+static void init_global_link_mode_masks(void)
{
- u32 mask = ep->supported;
+ static const enum ethtool_link_mode_bit_indices
+ all_advertised_modes_bits[] = {
+ ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+ ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+ ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+ ETHTOOL_LINK_MODE_2500baseX_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseR_FEC_BIT,
+ ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT,
+ ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT,
+ ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+ ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
+ ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT,
+ ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT,
+ ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT,
+ ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT,
+ ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT,
+ ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT,
+ };
+ static const enum ethtool_link_mode_bit_indices
+ additional_advertised_flags_bits[] = {
+ ETHTOOL_LINK_MODE_Autoneg_BIT,
+ ETHTOOL_LINK_MODE_TP_BIT,
+ ETHTOOL_LINK_MODE_AUI_BIT,
+ ETHTOOL_LINK_MODE_MII_BIT,
+ ETHTOOL_LINK_MODE_FIBRE_BIT,
+ ETHTOOL_LINK_MODE_BNC_BIT,
+ ETHTOOL_LINK_MODE_Pause_BIT,
+ ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ ETHTOOL_LINK_MODE_Backplane_BIT,
+ };
+ unsigned int i;
+ ethtool_link_mode_zero(all_advertised_modes);
+ ethtool_link_mode_zero(all_advertised_flags);
+ for (i = 0 ; i < ARRAY_SIZE(all_advertised_modes_bits) ; ++i) {
+ ethtool_link_mode_set_bit(all_advertised_modes_bits[i],
+ all_advertised_modes);
+ ethtool_link_mode_set_bit(all_advertised_modes_bits[i],
+ all_advertised_flags);
+ }
+
+ for (i = 0 ; i < ARRAY_SIZE(additional_advertised_flags_bits) ; ++i) {
+ ethtool_link_mode_set_bit(
+ additional_advertised_flags_bits[i],
+ all_advertised_flags);
+ }
+}
+
+static void dump_link_caps(const char *prefix, const char *an_prefix,
+ const u32 *mask, int link_mode_only);
+
+static void dump_supported(const struct ethtool_link_usettings *link_usettings)
+{
fprintf(stdout, " Supported ports: [ ");
- if (mask & SUPPORTED_TP)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_TP_BIT,
+ link_usettings->link_modes.supported))
fprintf(stdout, "TP ");
- if (mask & SUPPORTED_AUI)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_AUI_BIT,
+ link_usettings->link_modes.supported))
fprintf(stdout, "AUI ");
- if (mask & SUPPORTED_BNC)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_BNC_BIT,
+ link_usettings->link_modes.supported))
fprintf(stdout, "BNC ");
- if (mask & SUPPORTED_MII)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_MII_BIT,
+ link_usettings->link_modes.supported))
fprintf(stdout, "MII ");
- if (mask & SUPPORTED_FIBRE)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_FIBRE_BIT,
+ link_usettings->link_modes.supported))
fprintf(stdout, "FIBRE ");
- if (mask & SUPPORTED_Backplane)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_Backplane_BIT,
+ link_usettings->link_modes.supported))
fprintf(stdout, "Backplane ");
fprintf(stdout, "]\n");
- dump_link_caps("Supported", "Supports", mask, 0);
+ dump_link_caps("Supported", "Supports",
+ link_usettings->link_modes.supported, 0);
}
/* Print link capability flags (supported, advertised or lp_advertised).
* Assumes that the corresponding SUPPORTED and ADVERTISED flags are equal.
*/
-static void
-dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
- int link_mode_only)
+static void dump_link_caps(const char *prefix, const char *an_prefix,
+ const u32 *mask, int link_mode_only)
{
static const struct {
int same_line; /* print on same line as previous */
- u32 value;
+ unsigned int bit_index;
const char *name;
} mode_defs[] = {
- { 0, ADVERTISED_10baseT_Half, "10baseT/Half" },
- { 1, ADVERTISED_10baseT_Full, "10baseT/Full" },
- { 0, ADVERTISED_100baseT_Half, "100baseT/Half" },
- { 1, ADVERTISED_100baseT_Full, "100baseT/Full" },
- { 0, ADVERTISED_1000baseT_Half, "1000baseT/Half" },
- { 1, ADVERTISED_1000baseT_Full, "1000baseT/Full" },
- { 0, ADVERTISED_1000baseKX_Full, "1000baseKX/Full" },
- { 0, ADVERTISED_2500baseX_Full, "2500baseX/Full" },
- { 0, ADVERTISED_10000baseT_Full, "10000baseT/Full" },
- { 0, ADVERTISED_10000baseKX4_Full, "10000baseKX4/Full" },
- { 0, ADVERTISED_10000baseKR_Full, "10000baseKR/Full" },
- { 0, ADVERTISED_20000baseMLD2_Full, "20000baseMLD2/Full" },
- { 0, ADVERTISED_20000baseKR2_Full, "20000baseKR2/Full" },
- { 0, ADVERTISED_40000baseKR4_Full, "40000baseKR4/Full" },
- { 0, ADVERTISED_40000baseCR4_Full, "40000baseCR4/Full" },
- { 0, ADVERTISED_40000baseSR4_Full, "40000baseSR4/Full" },
- { 0, ADVERTISED_40000baseLR4_Full, "40000baseLR4/Full" },
- { 0, ADVERTISED_56000baseKR4_Full, "56000baseKR4/Full" },
- { 0, ADVERTISED_56000baseCR4_Full, "56000baseCR4/Full" },
- { 0, ADVERTISED_56000baseSR4_Full, "56000baseSR4/Full" },
- { 0, ADVERTISED_56000baseLR4_Full, "56000baseLR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+ "10baseT/Half" },
+ { 1, ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+ "10baseT/Full" },
+ { 0, ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+ "100baseT/Half" },
+ { 1, ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+ "100baseT/Full" },
+ { 0, ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+ "1000baseT/Half" },
+ { 1, ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ "1000baseT/Full" },
+ { 0, ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+ "1000baseKX/Full" },
+ { 0, ETHTOOL_LINK_MODE_2500baseX_Full_BIT,
+ "2500baseX/Full" },
+ { 0, ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ "10000baseT/Full" },
+ { 0, ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ "10000baseKX4/Full" },
+ { 0, ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ "10000baseKR/Full" },
+ { 0, ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT,
+ "20000baseMLD2/Full" },
+ { 0, ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT,
+ "20000baseKR2/Full" },
+ { 0, ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+ "40000baseKR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
+ "40000baseCR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT,
+ "40000baseSR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT,
+ "40000baseLR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT,
+ "56000baseKR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT,
+ "56000baseCR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT,
+ "56000baseSR4/Full" },
+ { 0, ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT,
+ "56000baseLR4/Full" },
};
int indent;
int did1, new_line_pend, i;
@@ -546,7 +640,8 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
for (i = 0; i < ARRAY_SIZE(mode_defs); i++) {
if (did1 && !mode_defs[i].same_line)
new_line_pend = 1;
- if (mask & mode_defs[i].value) {
+ if (ethtool_link_mode_test_bit(mode_defs[i].bit_index,
+ mask)) {
if (new_line_pend) {
fprintf(stdout, "\n");
fprintf(stdout, " %*s", indent, "");
@@ -562,46 +657,52 @@ dump_link_caps(const char *prefix, const char *an_prefix, u32 mask,
if (!link_mode_only) {
fprintf(stdout, " %s pause frame use: ", prefix);
- if (mask & ADVERTISED_Pause) {
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_Pause_BIT, mask)) {
fprintf(stdout, "Symmetric");
- if (mask & ADVERTISED_Asym_Pause)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask))
fprintf(stdout, " Receive-only");
fprintf(stdout, "\n");
} else {
- if (mask & ADVERTISED_Asym_Pause)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask))
fprintf(stdout, "Transmit-only\n");
else
fprintf(stdout, "No\n");
}
fprintf(stdout, " %s auto-negotiation: ", an_prefix);
- if (mask & ADVERTISED_Autoneg)
+ if (ethtool_link_mode_test_bit(
+ ETHTOOL_LINK_MODE_Autoneg_BIT, mask))
fprintf(stdout, "Yes\n");
else
fprintf(stdout, "No\n");
}
}
-static int dump_ecmd(struct ethtool_cmd *ep)
+static int
+dump_link_usettings(const struct ethtool_link_usettings *link_usettings)
{
- u32 speed;
-
- dump_supported(ep);
- dump_link_caps("Advertised", "Advertised", ep->advertising, 0);
- if (ep->lp_advertising)
+ dump_supported(link_usettings);
+ dump_link_caps("Advertised", "Advertised",
+ link_usettings->link_modes.advertising, 0);
+ if (!ethtool_link_mode_is_empty(
+ link_usettings->link_modes.lp_advertising))
dump_link_caps("Link partner advertised",
- "Link partner advertised", ep->lp_advertising,
- 0);
+ "Link partner advertised",
+ link_usettings->link_modes.lp_advertising, 0);
fprintf(stdout, " Speed: ");
- speed = ethtool_cmd_speed(ep);
- if (speed == 0 || speed == (u16)(-1) || speed == (u32)(-1))
+ if (link_usettings->base.speed == 0
+ || link_usettings->base.speed == (u16)(-1)
+ || link_usettings->base.speed == (u32)(-1))
fprintf(stdout, "Unknown!\n");
else
- fprintf(stdout, "%uMb/s\n", speed);
+ fprintf(stdout, "%uMb/s\n", link_usettings->base.speed);
fprintf(stdout, " Duplex: ");
- switch (ep->duplex) {
+ switch (link_usettings->base.duplex) {
case DUPLEX_HALF:
fprintf(stdout, "Half\n");
break;
@@ -609,12 +710,12 @@ static int dump_ecmd(struct ethtool_cmd *ep)
fprintf(stdout, "Full\n");
break;
default:
- fprintf(stdout, "Unknown! (%i)\n", ep->duplex);
+ fprintf(stdout, "Unknown! (%i)\n", link_usettings->base.duplex);
break;
};
fprintf(stdout, " Port: ");
- switch (ep->port) {
+ switch (link_usettings->base.port) {
case PORT_TP:
fprintf(stdout, "Twisted Pair\n");
break;
@@ -640,13 +741,13 @@ static int dump_ecmd(struct ethtool_cmd *ep)
fprintf(stdout, "Other\n");
break;
default:
- fprintf(stdout, "Unknown! (%i)\n", ep->port);
+ fprintf(stdout, "Unknown! (%i)\n", link_usettings->base.port);
break;
};
- fprintf(stdout, " PHYAD: %d\n", ep->phy_address);
+ fprintf(stdout, " PHYAD: %d\n", link_usettings->base.phy_address);
fprintf(stdout, " Transceiver: ");
- switch (ep->transceiver) {
+ switch (link_usettings->deprecated.transceiver) {
case XCVR_INTERNAL:
fprintf(stdout, "internal\n");
break;
@@ -659,17 +760,18 @@ static int dump_ecmd(struct ethtool_cmd *ep)
};
fprintf(stdout, " Auto-negotiation: %s\n",
- (ep->autoneg == AUTONEG_DISABLE) ?
+ (link_usettings->base.autoneg == AUTONEG_DISABLE) ?
"off" : "on");
- if (ep->port == PORT_TP) {
+ if (link_usettings->base.port == PORT_TP) {
fprintf(stdout, " MDI-X: ");
- if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI) {
+ if (link_usettings->base.eth_tp_mdix_ctrl == ETH_TP_MDI) {
fprintf(stdout, "off (forced)\n");
- } else if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_X) {
+ } else if (link_usettings->base.eth_tp_mdix_ctrl
+ == ETH_TP_MDI_X) {
fprintf(stdout, "on (forced)\n");
} else {
- switch (ep->eth_tp_mdix) {
+ switch (link_usettings->base.eth_tp_mdix) {
case ETH_TP_MDI:
fprintf(stdout, "off");
break;
@@ -680,7 +782,8 @@ static int dump_ecmd(struct ethtool_cmd *ep)
fprintf(stdout, "Unknown");
break;
}
- if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+ if (link_usettings->base.eth_tp_mdix_ctrl
+ == ETH_TP_MDI_AUTO)
fprintf(stdout, " (auto)");
fprintf(stdout, "\n");
}
@@ -1368,6 +1471,7 @@ static int dump_rxfhash(int fhash, u64 val)
static void dump_eeecmd(struct ethtool_eee *ep)
{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(link_mode);
fprintf(stdout, " EEE status: ");
if (!ep->supported) {
@@ -1389,9 +1493,16 @@ static void dump_eeecmd(struct ethtool_eee *ep)
else
fprintf(stdout, " disabled\n");
- dump_link_caps("Supported EEE", "", ep->supported, 1);
- dump_link_caps("Advertised EEE", "", ep->advertised, 1);
- dump_link_caps("Link partner advertised EEE", "", ep->lp_advertised, 1);
+ ethtool_link_mode_zero(link_mode);
+
+ link_mode[0] = ep->supported;
+ dump_link_caps("Supported EEE", "", link_mode, 1);
+
+ link_mode[0] = ep->advertised;
+ dump_link_caps("Advertised EEE", "", link_mode, 1);
+
+ link_mode[0] = ep->lp_advertised;
+ dump_link_caps("Link partner advertised EEE", "", link_mode, 1);
}
#define N_SOTS 7
@@ -2247,10 +2358,220 @@ static int do_sfeatures(struct cmd_context *ctx)
return 0;
}
-static int do_gset(struct cmd_context *ctx)
+static struct ethtool_link_usettings *
+__do_ioctl_glinksettings(struct cmd_context *ctx)
+{
+ int err;
+ struct {
+ struct ethtool_link_settings req;
+ __u32 link_mode_data[3*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
+ } ecmd;
+ struct ethtool_link_usettings *link_usettings;
+ unsigned int u32_offs;
+
+ /* Handshake with kernel to determine number of words for link
+ * mode bitmaps. When requested number of bitmap words is not
+ * the one expected by kernel, the latter returns the integer
+ * opposite of what it is expecting. We request length 0 below
+ * (aka. invalid bitmap length) to get this info.
+ */
+ memset(&ecmd, 0, sizeof(ecmd));
+ ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
+ err = send_ioctl(ctx, &ecmd);
+ if (err < 0)
+ return NULL;
+
+ /* see above: we expect a strictly negative value from kernel.
+ */
+ if (ecmd.req.link_mode_masks_nwords >= 0
+ || ecmd.req.cmd != ETHTOOL_GLINKSETTINGS)
+ return NULL;
+
+ /* got the real ecmd.req.link_mode_masks_nwords,
+ * now send the real request
+ */
+ ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
+ ecmd.req.link_mode_masks_nwords = -ecmd.req.link_mode_masks_nwords;
+ err = send_ioctl(ctx, &ecmd);
+ if (err < 0)
+ return NULL;
+
+ if (ecmd.req.link_mode_masks_nwords <= 0
+ || ecmd.req.cmd != ETHTOOL_GLINKSETTINGS)
+ return NULL;
+
+ /* Convert to usettings struct */
+ link_usettings = calloc(1, sizeof(*link_usettings));
+ if (link_usettings == NULL)
+ return NULL;
+
+ /* keep transceiver 0 */
+ memcpy(&link_usettings->base, &ecmd.req, sizeof(link_usettings->base));
+
+ /* copy link mode bitmaps */
+ u32_offs = 0;
+ memcpy(link_usettings->link_modes.supported,
+ &ecmd.link_mode_data[u32_offs],
+ 4*ecmd.req.link_mode_masks_nwords);
+
+ u32_offs += ecmd.req.link_mode_masks_nwords;
+ memcpy(link_usettings->link_modes.advertising,
+ &ecmd.link_mode_data[u32_offs],
+ 4*ecmd.req.link_mode_masks_nwords);
+
+ u32_offs += ecmd.req.link_mode_masks_nwords;
+ memcpy(link_usettings->link_modes.lp_advertising,
+ &ecmd.link_mode_data[u32_offs],
+ 4*ecmd.req.link_mode_masks_nwords);
+
+ return link_usettings;
+}
+
+static int
+__do_ioctl_slinksettings(struct cmd_context *ctx,
+ const struct ethtool_link_usettings *link_usettings)
+{
+ struct {
+ struct ethtool_link_settings req;
+ __u32 link_mode_data[3*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
+ } ecmd;
+ unsigned int u32_offs;
+
+ /* refuse to send ETHTOOL_SLINKSETTINGS ioctl if
+ * link_usettings was retrieved with ETHTOOL_GSET
+ */
+ if (link_usettings->base.cmd != ETHTOOL_GLINKSETTINGS)
+ return -1;
+
+ /* refuse to send ETHTOOL_SLINKSETTINGS ioctl if deprecated fields
+ * were set
+ */
+ if (link_usettings->deprecated.transceiver)
+ return -1;
+
+ if (link_usettings->base.link_mode_masks_nwords <= 0)
+ return -1;
+
+ memcpy(&ecmd.req, &link_usettings->base, sizeof(ecmd.req));
+ ecmd.req.cmd = ETHTOOL_SLINKSETTINGS;
+
+ /* copy link mode bitmaps */
+ u32_offs = 0;
+ memcpy(&ecmd.link_mode_data[u32_offs],
+ link_usettings->link_modes.supported,
+ 4*ecmd.req.link_mode_masks_nwords);
+
+ u32_offs += ecmd.req.link_mode_masks_nwords;
+ memcpy(&ecmd.link_mode_data[u32_offs],
+ link_usettings->link_modes.advertising,
+ 4*ecmd.req.link_mode_masks_nwords);
+
+ u32_offs += ecmd.req.link_mode_masks_nwords;
+ memcpy(&ecmd.link_mode_data[u32_offs],
+ link_usettings->link_modes.lp_advertising,
+ 4*ecmd.req.link_mode_masks_nwords);
+
+ return send_ioctl(ctx, &ecmd);
+}
+
+static struct ethtool_link_usettings *
+__do_ioctl_gset(struct cmd_context *ctx)
{
int err;
struct ethtool_cmd ecmd;
+ struct ethtool_link_usettings *link_usettings;
+
+ memset(&ecmd, 0, sizeof(ecmd));
+ ecmd.cmd = ETHTOOL_GSET;
+ err = send_ioctl(ctx, &ecmd);
+ if (err < 0)
+ return NULL;
+
+ link_usettings = calloc(1, sizeof(*link_usettings));
+ if (link_usettings == NULL)
+ return NULL;
+
+ /* remember that ETHTOOL_GSET was used */
+ link_usettings->base.cmd = ETHTOOL_GSET;
+
+ link_usettings->base.link_mode_masks_nwords = 1;
+ link_usettings->link_modes.supported[0] = ecmd.supported;
+ link_usettings->link_modes.advertising[0] = ecmd.advertising;
+ link_usettings->link_modes.lp_advertising[0] = ecmd.lp_advertising;
+ link_usettings->base.speed = ethtool_cmd_speed(&ecmd);
+ link_usettings->base.duplex = ecmd.duplex;
+ link_usettings->base.port = ecmd.port;
+ link_usettings->base.phy_address = ecmd.phy_address;
+ link_usettings->deprecated.transceiver = ecmd.transceiver;
+ link_usettings->base.autoneg = ecmd.autoneg;
+ link_usettings->base.mdio_support = ecmd.mdio_support;
+ /* ignored (fully deprecated): maxrxpkt, maxtxpkt */
+ link_usettings->base.eth_tp_mdix = ecmd.eth_tp_mdix;
+ link_usettings->base.eth_tp_mdix_ctrl = ecmd.eth_tp_mdix_ctrl;
+
+ return link_usettings;
+}
+
+static bool ethtool_link_mode_is_backward_compatible(const u32 *mask)
+{
+ unsigned int i;
+
+ for (i = 1 ; i < __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32 ; ++i)
+ if (mask[i])
+ return false;
+
+ return true;
+}
+
+static int
+__do_ioctl_sset(struct cmd_context *ctx,
+ const struct ethtool_link_usettings *link_usettings)
+{
+ struct ethtool_cmd ecmd;
+
+ /* refuse to send ETHTOOL_SSET ioctl if link_usettings was
+ * retrieved with ETHTOOL_GLINKSETTINGS
+ */
+ if (link_usettings->base.cmd != ETHTOOL_GSET)
+ return -1;
+
+ if (link_usettings->base.link_mode_masks_nwords <= 0)
+ return -1;
+
+ /* refuse to sset if any bit > 31 is set */
+ if (!ethtool_link_mode_is_backward_compatible(
+ link_usettings->link_modes.supported))
+ return -1;
+ if (!ethtool_link_mode_is_backward_compatible(
+ link_usettings->link_modes.advertising))
+ return -1;
+ if (!ethtool_link_mode_is_backward_compatible(
+ link_usettings->link_modes.lp_advertising))
+ return -1;
+
+ memset(&ecmd, 0, sizeof(ecmd));
+ ecmd.cmd = ETHTOOL_SSET;
+
+ ecmd.supported = link_usettings->link_modes.supported[0];
+ ecmd.advertising = link_usettings->link_modes.advertising[0];
+ ecmd.lp_advertising = link_usettings->link_modes.lp_advertising[0];
+ ethtool_cmd_speed_set(&ecmd, link_usettings->base.speed);
+ ecmd.duplex = link_usettings->base.duplex;
+ ecmd.port = link_usettings->base.port;
+ ecmd.phy_address = link_usettings->base.phy_address;
+ ecmd.transceiver = link_usettings->deprecated.transceiver;
+ ecmd.autoneg = link_usettings->base.autoneg;
+ ecmd.mdio_support = link_usettings->base.mdio_support;
+ /* ignored (fully deprecated): maxrxpkt, maxtxpkt */
+ ecmd.eth_tp_mdix = link_usettings->base.eth_tp_mdix;
+ ecmd.eth_tp_mdix_ctrl = link_usettings->base.eth_tp_mdix_ctrl;
+ return send_ioctl(ctx, &ecmd);
+}
+
+static int do_gset(struct cmd_context *ctx)
+{
+ int err;
+ struct ethtool_link_usettings *link_usettings;
struct ethtool_wolinfo wolinfo;
struct ethtool_value edata;
int allfail = 1;
@@ -2260,10 +2581,12 @@ static int do_gset(struct cmd_context *ctx)
fprintf(stdout, "Settings for %s:\n", ctx->devname);
- ecmd.cmd = ETHTOOL_GSET;
- err = send_ioctl(ctx, &ecmd);
- if (err == 0) {
- err = dump_ecmd(&ecmd);
+ link_usettings = __do_ioctl_glinksettings(ctx);
+ if (link_usettings == NULL)
+ link_usettings = __do_ioctl_gset(ctx);
+ if (link_usettings != NULL) {
+ err = dump_link_usettings(link_usettings);
+ free(link_usettings);
if (err)
return err;
allfail = 0;
@@ -2322,8 +2645,10 @@ static int do_sset(struct cmd_context *ctx)
int autoneg_wanted = -1;
int phyad_wanted = -1;
int xcvr_wanted = -1;
- int full_advertising_wanted = -1;
- int advertising_wanted = -1;
+ u32 *full_advertising_wanted = NULL;
+ u32 *advertising_wanted = NULL;
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(_mask_full_advertising_wanted);
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(_mask_advertising_wanted);
int gset_changed = 0; /* did anything in GSET change? */
u32 wol_wanted = 0;
int wol_change = 0;
@@ -2337,7 +2662,7 @@ static int do_sset(struct cmd_context *ctx)
int argc = ctx->argc;
char **argp = ctx->argp;
int i;
- int err;
+ int err = 0;
for (i = 0; i < ARRAY_SIZE(flags_msglvl); i++)
flag_to_cmdline_info(flags_msglvl[i].name,
@@ -2411,7 +2736,12 @@ static int do_sset(struct cmd_context *ctx)
i += 1;
if (i >= argc)
exit_bad_args();
- full_advertising_wanted = get_int(argp[i], 16);
+ if (parse_hex_u32_bitmap(
+ argp[i],
+ __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS,
+ _mask_full_advertising_wanted))
+ exit_bad_args();
+ full_advertising_wanted = _mask_full_advertising_wanted;
} else if (!strcmp(argp[i], "phyad")) {
gset_changed = 1;
i += 1;
@@ -2468,75 +2798,89 @@ static int do_sset(struct cmd_context *ctx)
}
}
- if (full_advertising_wanted < 0) {
+ if (full_advertising_wanted == NULL) {
/* User didn't supply a full advertisement bitfield:
* construct one from the specified speed and duplex.
*/
+ int adv_bit = -1;
+
if (speed_wanted == SPEED_10 && duplex_wanted == DUPLEX_HALF)
- advertising_wanted = ADVERTISED_10baseT_Half;
+ adv_bit = ETHTOOL_LINK_MODE_10baseT_Half_BIT;
else if (speed_wanted == SPEED_10 &&
duplex_wanted == DUPLEX_FULL)
- advertising_wanted = ADVERTISED_10baseT_Full;
+ adv_bit = ETHTOOL_LINK_MODE_10baseT_Full_BIT;
else if (speed_wanted == SPEED_100 &&
duplex_wanted == DUPLEX_HALF)
- advertising_wanted = ADVERTISED_100baseT_Half;
+ adv_bit = ETHTOOL_LINK_MODE_100baseT_Half_BIT;
else if (speed_wanted == SPEED_100 &&
duplex_wanted == DUPLEX_FULL)
- advertising_wanted = ADVERTISED_100baseT_Full;
+ adv_bit = ETHTOOL_LINK_MODE_100baseT_Full_BIT;
else if (speed_wanted == SPEED_1000 &&
duplex_wanted == DUPLEX_HALF)
- advertising_wanted = ADVERTISED_1000baseT_Half;
+ adv_bit = ETHTOOL_LINK_MODE_1000baseT_Half_BIT;
else if (speed_wanted == SPEED_1000 &&
duplex_wanted == DUPLEX_FULL)
- advertising_wanted = ADVERTISED_1000baseT_Full;
+ adv_bit = ETHTOOL_LINK_MODE_1000baseT_Full_BIT;
else if (speed_wanted == SPEED_2500 &&
duplex_wanted == DUPLEX_FULL)
- advertising_wanted = ADVERTISED_2500baseX_Full;
+ adv_bit = ETHTOOL_LINK_MODE_2500baseX_Full_BIT;
else if (speed_wanted == SPEED_10000 &&
duplex_wanted == DUPLEX_FULL)
- advertising_wanted = ADVERTISED_10000baseT_Full;
- else
- /* auto negotiate without forcing,
- * all supported speed will be assigned below
- */
- advertising_wanted = 0;
+ adv_bit = ETHTOOL_LINK_MODE_10000baseT_Full_BIT;
+
+ if (adv_bit >= 0) {
+ advertising_wanted = _mask_advertising_wanted;
+ ethtool_link_mode_zero(advertising_wanted);
+ ethtool_link_mode_set_bit(
+ adv_bit, advertising_wanted);
+ }
+ /* otherwise: auto negotiate without forcing,
+ * all supported speed will be assigned below
+ */
}
if (gset_changed) {
- struct ethtool_cmd ecmd;
+ struct ethtool_link_usettings *link_usettings;
- ecmd.cmd = ETHTOOL_GSET;
- err = send_ioctl(ctx, &ecmd);
- if (err < 0) {
+ link_usettings = __do_ioctl_glinksettings(ctx);
+ if (link_usettings == NULL)
+ link_usettings = __do_ioctl_gset(ctx);
+ if (link_usettings == NULL) {
perror("Cannot get current device settings");
+ err = -1;
} else {
/* Change everything the user specified. */
if (speed_wanted != -1)
- ethtool_cmd_speed_set(&ecmd, speed_wanted);
+ link_usettings->base.speed = speed_wanted;
if (duplex_wanted != -1)
- ecmd.duplex = duplex_wanted;
+ link_usettings->base.duplex = duplex_wanted;
if (port_wanted != -1)
- ecmd.port = port_wanted;
+ link_usettings->base.port = port_wanted;
if (mdix_wanted != -1) {
/* check driver supports MDI-X */
- if (ecmd.eth_tp_mdix_ctrl != ETH_TP_MDI_INVALID)
- ecmd.eth_tp_mdix_ctrl = mdix_wanted;
+ if (link_usettings->base.eth_tp_mdix_ctrl
+ != ETH_TP_MDI_INVALID)
+ link_usettings->base.eth_tp_mdix_ctrl
+ = mdix_wanted;
else
- fprintf(stderr, "setting MDI not supported\n");
+ fprintf(stderr,
+ "setting MDI not supported\n");
}
if (autoneg_wanted != -1)
- ecmd.autoneg = autoneg_wanted;
+ link_usettings->base.autoneg = autoneg_wanted;
if (phyad_wanted != -1)
- ecmd.phy_address = phyad_wanted;
+ link_usettings->base.phy_address = phyad_wanted;
if (xcvr_wanted != -1)
- ecmd.transceiver = xcvr_wanted;
+ link_usettings->deprecated.transceiver
+ = xcvr_wanted;
/* XXX If the user specified speed or duplex
* then we should mask the advertised modes
* accordingly. For now, warn that we aren't
* doing that.
*/
if ((speed_wanted != -1 || duplex_wanted != -1) &&
- ecmd.autoneg && advertising_wanted == 0) {
+ link_usettings->base.autoneg &&
+ advertising_wanted == NULL) {
fprintf(stderr, "Cannot advertise");
if (speed_wanted >= 0)
fprintf(stderr, " speed %d",
@@ -2548,37 +2892,56 @@ static int do_sset(struct cmd_context *ctx)
fprintf(stderr, "\n");
}
if (autoneg_wanted == AUTONEG_ENABLE &&
- advertising_wanted == 0) {
+ advertising_wanted == NULL) {
+ unsigned int i;
+
/* Auto negotiation enabled, but with
* unspecified speed and duplex: enable all
* supported speeds and duplexes.
*/
- ecmd.advertising =
- (ecmd.advertising &
- ~ALL_ADVERTISED_MODES) |
- (ALL_ADVERTISED_MODES & ecmd.supported);
+ ethtool_link_mode_for_each_u32(i) {
+ u32 sup = link_usettings->link_modes.supported[i];
+ u32 *adv = link_usettings->link_modes.advertising + i;
+
+ *adv = ((*adv & ~all_advertised_modes[i])
+ | (sup & all_advertised_modes[i]));
+ }
/* If driver supports unknown flags, we cannot
* be sure that we enable all link modes.
*/
- if ((ecmd.supported & ALL_ADVERTISED_FLAGS) !=
- ecmd.supported) {
- fprintf(stderr, "Driver supports one "
- "or more unknown flags\n");
+ ethtool_link_mode_for_each_u32(i) {
+ u32 sup = link_usettings->link_modes.supported[i];
+
+ if ((sup & all_advertised_flags[i]) != sup) {
+ fprintf(stderr, "Driver supports one or more unknown flags\n");
+ break;
+ }
}
- } else if (advertising_wanted > 0) {
+ } else if (advertising_wanted != NULL) {
+ unsigned int i;
+
/* Enable all requested modes */
- ecmd.advertising =
- (ecmd.advertising &
- ~ALL_ADVERTISED_MODES) |
- advertising_wanted;
- } else if (full_advertising_wanted > 0) {
- ecmd.advertising = full_advertising_wanted;
+ ethtool_link_mode_for_each_u32(i) {
+ u32 *adv = link_usettings->link_modes.advertising + i;
+
+ *adv = ((*adv & ~all_advertised_modes[i])
+ | advertising_wanted[i]);
+ }
+ } else if (full_advertising_wanted != NULL) {
+ ethtool_link_mode_copy(
+ link_usettings->link_modes.advertising,
+ full_advertising_wanted);
}
/* Try to perform the update. */
- ecmd.cmd = ETHTOOL_SSET;
- err = send_ioctl(ctx, &ecmd);
+ if (link_usettings->base.cmd == ETHTOOL_GLINKSETTINGS)
+ err = __do_ioctl_slinksettings(ctx,
+ link_usettings);
+ else
+ err = __do_ioctl_sset(ctx,
+ link_usettings);
+ free(link_usettings);
if (err < 0)
perror("Cannot set new settings");
}
@@ -4230,6 +4593,8 @@ int main(int argc, char **argp)
struct cmd_context ctx;
int k;
+ init_global_link_mode_masks();
+
/* Skip command name */
argp++;
argc--;
diff --git a/internal.h b/internal.h
index e98f532..5e78b5c 100644
--- a/internal.h
+++ b/internal.h
@@ -12,6 +12,7 @@
#ifdef HAVE_CONFIG_H
#include "ethtool-config.h"
#endif
+#include <stdbool.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
@@ -122,6 +123,72 @@ static inline int test_bit(unsigned int nr, const unsigned long *addr)
ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE | \
ETH_FLAG_RXHASH)
+/* internal API for link mode bitmap interaction with kernel. */
+
+#define __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32 \
+ (SCHAR_MAX)
+#define __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS \
+ (32*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32)
+#define __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES \
+ (4*__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32)
+#define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \
+ u32 name[__ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32]
+
+struct ethtool_link_usettings {
+ struct {
+ __u8 transceiver;
+ } deprecated;
+ struct ethtool_link_settings base;
+ struct {
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
+ } link_modes;
+};
+
+#define ethtool_link_mode_for_each_u32(index) \
+ for ((index) = 0 ; \
+ (index) < __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32 ; \
+ ++(index))
+
+static inline void ethtool_link_mode_zero(u32 *dst)
+{
+ memset(dst, 0, __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES);
+}
+
+static inline bool ethtool_link_mode_is_empty(const u32 *mask)
+{
+ unsigned int i;
+
+ ethtool_link_mode_for_each_u32(i)
+ {
+ if (mask[i] != 0)
+ return false;
+ }
+
+ return true;
+}
+
+static inline void ethtool_link_mode_copy(u32 *dst, const u32 *src)
+{
+ memcpy(dst, src, __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES);
+}
+
+static inline int ethtool_link_mode_test_bit(unsigned int nr, const u32 *mask)
+{
+ if (nr >= __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS)
+ return !!0;
+ return !!(mask[nr / 32] & (1 << (nr % 32)));
+}
+
+static inline int ethtool_link_mode_set_bit(unsigned int nr, u32 *mask)
+{
+ if (nr >= __ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS)
+ return -1;
+ mask[nr / 32] |= (1 << (nr % 32));
+ return 0;
+}
+
/* Context for sub-commands */
struct cmd_context {
const char *devname; /* net device name */
diff --git a/test-cmdline.c b/test-cmdline.c
index 2fd7cbb..a94edea 100644
--- a/test-cmdline.c
+++ b/test-cmdline.c
@@ -37,7 +37,20 @@ static struct test_case {
{ 1, "--change devname autoneg foo" },
{ 1, "-s devname autoneg" },
{ 0, "--change devname advertise 0x1" },
+ { 0, "--change devname advertise 0xf" },
+ { 0, "--change devname advertise 0Xf" },
+ { 0, "--change devname advertise 1" },
+ { 0, "--change devname advertise f" },
+ { 0, "--change devname advertise 01" },
+ { 0, "--change devname advertise 0f" },
+ { 0, "--change devname advertise 0xfffffffffffffffffffffffffffffffff" },
+ { 0, "--change devname advertise fffffffffffffffffffffffffffffffff" },
+ { 0, "--change devname advertise 0x0000fffffffffffffffffffffffffffff" },
+ { 0, "--change devname advertise 0000fffffffffffffffffffffffffffff" },
+ { 1, "-s devname advertise" },
+ { 1, "-s devname advertise 0x" },
{ 1, "-s devname advertise foo" },
+ { 1, "-s devname advertise 0xfoo" },
{ 1, "--change devname advertise" },
{ 0, "-s devname phyad 1" },
{ 1, "--change devname phyad foo" },
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [ethtool PATCH v7 2/2] ethtool: use netlink socket when AF_INET not available
From: David Decotigny @ 2016-03-25 16:21 UTC (permalink / raw)
To: netdev
Cc: Jeff Garzik, Ben Hutchings, David Miller, Vidya Sagar Ravipati,
Joe Perches, David Decotigny
In-Reply-To: <1458922862-98024-1-git-send-email-ddecotig@gmail.com>
From: David Decotigny <decot@googlers.com>
To benefit from this, kernel commit 025c68186e07 ("netlink: add support
for NIC driver ioctls") is needed.
Signed-off-by: David Decotigny <decot@googlers.com>
---
configure.ac | 2 +-
ethtool.c | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 3105415..47d2a0f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,7 +15,7 @@ AM_PROG_CC_C_O
dnl Checks for libraries.
dnl Checks for header files.
-AC_CHECK_HEADERS(sys/ioctl.h)
+AC_CHECK_HEADERS(sys/ioctl.h linux/netlink.h)
dnl Checks for typedefs, structures, and compiler characteristics.
AC_MSG_CHECKING([whether <linux/types.h> defines big-endian types])
diff --git a/ethtool.c b/ethtool.c
index cb3d971..314b1b8 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -42,6 +42,9 @@
#include <arpa/inet.h>
#include <linux/sockios.h>
+#ifdef HAVE_LINUX_NETLINK_H
+# include <linux/netlink.h>
+#endif
#ifndef MAX_ADDR_LEN
#define MAX_ADDR_LEN 32
@@ -4645,6 +4648,10 @@ opt_found:
/* Open control socket. */
ctx.fd = socket(AF_INET, SOCK_DGRAM, 0);
+#ifdef HAVE_LINUX_NETLINK_H
+ if (ctx.fd < 0)
+ ctx.fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
+#endif
if (ctx.fd < 0) {
perror("Cannot get control socket");
return 70;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Alexei Starovoitov @ 2016-03-25 16:21 UTC (permalink / raw)
To: Craig Gallek; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAEfhGizvwATcG--OH_xhrV-c1t11ie-vNibJY8kqFAq3GX9upw@mail.gmail.com>
On Fri, Mar 25, 2016 at 11:29:10AM -0400, Craig Gallek wrote:
> On Thu, Mar 24, 2016 at 2:00 PM, Willy Tarreau <w@1wt.eu> wrote:
> > The pattern is :
> >
> > t0 : unprivileged processes 1 and 2 are listening to the same port
> > (sock1@pid1) (sock2@pid2)
> > <------ listening ------>
> >
> > t1 : new processes are started to replace the old ones
> > (sock1@pid1) (sock2@pid2) (sock3@pid3) (sock4@pid4)
> > <------ listening ------> <------ listening ------>
> >
> > t2 : new processes signal the old ones they must stop
> > (sock1@pid1) (sock2@pid2) (sock3@pid3) (sock4@pid4)
> > <------- draining ------> <------ listening ------>
> >
> > t3 : pids 1 and 2 have finished, they go away
> > (sock3@pid3) (sock4@pid4)
> > <------ gone -----> <------ listening ------>
...
> t3: Close the first two sockets and only use the last two. This is
> the tricky step. Before this point, the sockets are numbered 0
> through 3 from the perspective of the BPF program (in the order
> listen() was called). As soon as socket 0 is closed, the last socket
> in the list replaces it (what was 3 becomes 0). When socket 1 is
> closed, socket 2 moves into that position. The assumptions about the
> socket indexes in the BPF program need to change as the indexes change
> as a result of closing them.
yeah, the way reuseport_detach_sock() was done makes it hard to manage
such transitions from bpf program, but I don't see yet what stops
pid1 an pid2 at stage t2 to just close their sockets.
If these 'draining' pids don't want to receive packets, they should
close their sockets. Complicating bpf side to redistribute spraying
to sock3 and sock4 only (while sock1 and sock2 are still open) is possible,
but looks unnecessary complex to me.
Just close sock1 and sock2 at t2 time and then exit pid1, pid2 later.
If they are tcp sockets with rpc protocol on top and have a problem of
partial messages, then kcm can solve that and it will simplify
the user space side as well.
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Craig Gallek @ 2016-03-25 16:31 UTC (permalink / raw)
To: Linux Kernel Network Developers; +Cc: Alexei Starovoitov
In-Reply-To: <20160325162114.GA72479@ast-mbp.thefacebook.com>
On Fri, Mar 25, 2016 at 12:21 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Fri, Mar 25, 2016 at 11:29:10AM -0400, Craig Gallek wrote:
>> On Thu, Mar 24, 2016 at 2:00 PM, Willy Tarreau <w@1wt.eu> wrote:
>> > The pattern is :
>> >
>> > t0 : unprivileged processes 1 and 2 are listening to the same port
>> > (sock1@pid1) (sock2@pid2)
>> > <------ listening ------>
>> >
>> > t1 : new processes are started to replace the old ones
>> > (sock1@pid1) (sock2@pid2) (sock3@pid3) (sock4@pid4)
>> > <------ listening ------> <------ listening ------>
>> >
>> > t2 : new processes signal the old ones they must stop
>> > (sock1@pid1) (sock2@pid2) (sock3@pid3) (sock4@pid4)
>> > <------- draining ------> <------ listening ------>
>> >
>> > t3 : pids 1 and 2 have finished, they go away
>> > (sock3@pid3) (sock4@pid4)
>> > <------ gone -----> <------ listening ------>
> ...
>> t3: Close the first two sockets and only use the last two. This is
>> the tricky step. Before this point, the sockets are numbered 0
>> through 3 from the perspective of the BPF program (in the order
>> listen() was called). As soon as socket 0 is closed, the last socket
>> in the list replaces it (what was 3 becomes 0). When socket 1 is
>> closed, socket 2 moves into that position. The assumptions about the
>> socket indexes in the BPF program need to change as the indexes change
>> as a result of closing them.
>
> yeah, the way reuseport_detach_sock() was done makes it hard to manage
> such transitions from bpf program, but I don't see yet what stops
> pid1 an pid2 at stage t2 to just close their sockets.
> If these 'draining' pids don't want to receive packets, they should
> close their sockets. Complicating bpf side to redistribute spraying
> to sock3 and sock4 only (while sock1 and sock2 are still open) is possible,
> but looks unnecessary complex to me.
> Just close sock1 and sock2 at t2 time and then exit pid1, pid2 later.
> If they are tcp sockets with rpc protocol on top and have a problem of
> partial messages, then kcm can solve that and it will simplify
> the user space side as well.
I believe the issue here is that closing the listen sockets will drop
any connections that are in the listen queue but have not been
accepted yet. In the case of reuseport, you could in theory drain
those queues into the non-closed sockets, but that probably has some
interesting consequences...
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: Cong Wang @ 2016-03-25 16:32 UTC (permalink / raw)
To: Ben Greear; +Cc: Vijay Pandurangan, netdev, Evan Jones, Cong Wang, Tom Herbert
In-Reply-To: <56F56312.4080408@candelatech.com>
On Fri, Mar 25, 2016 at 9:10 AM, Ben Greear <greearb@candelatech.com> wrote:
> I am suspicious that this will break at least some drivers. I grepped
> around
> for ip_summed, and found this, for instance:
>
> davicom/dm9000.c
>
> /* The DM9000 is not smart enough to leave fragmented packets alone.
> */
> if (dm->ip_summed != ip_summed) {
> if (ip_summed == CHECKSUM_NONE)
> iow(dm, DM9000_TCCR, 0);
> else
> iow(dm, DM9000_TCCR, TCCR_IP | TCCR_UDP | TCCR_TCP);
> dm->ip_summed = ip_summed;
> }
>
>
> It is taking action based on ip_summed == CHECKSUM_NONE, and your change
> will probably break that.
>
> I would suggest that we try to make any fix specific only to veth,
> at least for now. A tree-wide audit of drivers is probably required
> to safely make the kind of change you propose above.
>
> So, unless you can explain why your change is safe, then I do not plan
> to test it.
I just blindly trust the comments there:
* CHECKSUM_UNNECESSARY:
*
* This has the same meaning on as CHECKSUM_NONE for checksum offload on
* output.
Let's Cc Tom who wrote this comment.
On the other hand, hyperv got this correctly:
if ((skb->ip_summed == CHECKSUM_NONE) ||
(skb->ip_summed == CHECKSUM_UNNECESSARY))
goto do_send;
So I believe dm9000 needs to fix.
Thanks.
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: David Miller @ 2016-03-25 16:44 UTC (permalink / raw)
To: greearb; +Cc: xiyou.wangcong, vijayp, netdev, ej, cwang
In-Reply-To: <56F56312.4080408@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Date: Fri, 25 Mar 2016 09:10:58 -0700
> I am suspicious that this will break at least some drivers. I
> grepped around for ip_summed, and found this, for instance:
>
> davicom/dm9000.c
>
> /* The DM9000 is not smart enough to leave fragmented packets
An really old (circa 1997), not-oft-used, driver such as this is not
the place to be looking for correct usage of skb->ip_summed semantics.
I would never use whatever a driver like this does influence whether I
apply a bug fix or not.
^ permalink raw reply
* Re: veth regression with "don’t modify ip_summed; doing so treats packets with bad checksums as good."
From: David Miller @ 2016-03-25 16:45 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: greearb, vijayp, netdev, ej, cwang, tom
In-Reply-To: <CAM_iQpXpqVgweZFw438v_b7No8U8YtqHSt0x0+HZ+oSimdCyfg@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 25 Mar 2016 09:32:23 -0700
> So I believe dm9000 needs to fix.
+1
^ permalink raw reply
* Re: [PATCH v2] netpoll: Fix extra refcount release in netpoll_cleanup()
From: Bjorn Helgaas @ 2016-03-25 16:46 UTC (permalink / raw)
To: Neil Horman
Cc: Bjorn Helgaas, David S. Miller, Nikolay Aleksandrov, netdev,
Alexander Duyck, linux-kernel
In-Reply-To: <20160325113342.GA21579@hmsreliant.think-freely.org>
On Fri, Mar 25, 2016 at 07:33:42AM -0400, Neil Horman wrote:
> On Thu, Mar 24, 2016 at 09:56:21PM -0500, Bjorn Helgaas wrote:
> > netpoll_setup() does a dev_hold() on np->dev, the netpoll device. If it
> > fails, it correctly does a dev_put() but leaves np->dev set. If we call
> > netpoll_cleanup() after the failure, np->dev is still set so we do another
> > dev_put(), which decrements the refcount an extra time.
> >
> > It's questionable to call netpoll_cleanup() after netpoll_setup() fails,
> > but it can be difficult to find the problem, and we can easily avoid it in
> > this case. The extra decrements can lead to hangs like this:
> >
> > unregister_netdevice: waiting for bond0 to become free. Usage count = -3
> >
> > Set and clear np->dev at the points where we dev_hold() and dev_put() the
> > device.
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> > net/core/netpoll.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> > index 94acfc8..a57bd17 100644
> > --- a/net/core/netpoll.c
> > +++ b/net/core/netpoll.c
> > @@ -603,7 +603,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
> > const struct net_device_ops *ops;
> > int err;
> >
> > - np->dev = ndev;
> > strlcpy(np->dev_name, ndev->name, IFNAMSIZ);
> > INIT_WORK(&np->cleanup_work, netpoll_async_cleanup);
> >
> > @@ -670,6 +669,7 @@ int netpoll_setup(struct netpoll *np)
> > goto unlock;
> > }
> > dev_hold(ndev);
> > + np->dev = ndev;
> >
> > if (netdev_master_upper_dev_get(ndev)) {
> > np_err(np, "%s is a slave device, aborting\n", np->dev_name);
> > @@ -770,6 +770,7 @@ int netpoll_setup(struct netpoll *np)
> > return 0;
> >
> > put:
> > + np->dev = NULL;
> > dev_put(ndev);
> > unlock:
> > rtnl_unlock();
> >
>
> Is this safe for stacked devices? It makes good sense for the typical case, but
> if you attempt to setup a netpoll client on a bridge/bond/vlan, etc, the lower
> device will get its own netpoll struct registered and have no associated np->dev
> pointer. It not be a real problem in practice, But you probably want to check
> to make sure that stacked devices which recursively call the netpoll api don't
> do anyting with the np->dev pointer.
You're right, there is an issue here. I reproduced a problem with a
bond device. bond_netpoll_setup() calls __netpoll_setup() directly
(not netpoll_setup()). I'll debug it more; just wanted to let you
know there *is* a problem with this patch.
Bjorn
^ permalink raw reply
* Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode
From: Eric Dumazet @ 2016-03-25 17:00 UTC (permalink / raw)
To: Craig Gallek; +Cc: Linux Kernel Network Developers, Alexei Starovoitov
In-Reply-To: <CAEfhGiy-V0da6LQ_SY+E4u7S49n5dEpVaKiOOo103ijQtPujAw@mail.gmail.com>
On Fri, 2016-03-25 at 12:31 -0400, Craig Gallek wrote:
> I believe the issue here is that closing the listen sockets will drop
> any connections that are in the listen queue but have not been
> accepted yet. In the case of reuseport, you could in theory drain
> those queues into the non-closed sockets, but that probably has some
> interesting consequences...
It is more complicated than this.
Ideally, no TCP connection should be dropped during a server change.
The idea is to let old program running as long as :
1) It has established TCP sessions
2) Some SYN_RECV pseudo requests are still around
Once 3WHS completes for these SYN_RECV, children are queued into
listener accept queues.
But the idea is to direct all new SYN packets to the 'new' process and
its listeners. (New SYN_RECV should be created on behalf on the new
listeners only)
In some environments, the listeners are simply transfered via FD
passing, from the 'old process' to the new one.
^ 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