* RE: [PATCH v2 RESEND] xen-netback: prefer xenbus_scanf() over xenbus_gather()
From: Jan Beulich @ 2016-10-25 8:22 UTC (permalink / raw)
To: Paul Durrant
Cc: David Vrabel, Wei Liu, xen-devel@lists.xenproject.org,
boris.ostrovsky@oracle.com, Juergen Gross, netdev@vger.kernel.org
In-Reply-To: <8cc49b57d5d64a439ddc99b94c99f9a3@AMSPEX02CL03.citrite.net>
>>> On 25.10.16 at 09:52, <Paul.Durrant@citrix.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 24 October 2016 16:08
>> --- 4.9-rc2/drivers/net/xen-netback/xenbus.c
>> +++ 4.9-rc2-xen-netback-prefer-xenbus_scanf/drivers/net/xen-netback/xenbus.c
>> @@ -889,16 +889,16 @@ static int connect_ctrl_ring(struct back
>> unsigned int evtchn;
>> int err;
>>
>> - err = xenbus_gather(XBT_NIL, dev->otherend,
>> - "ctrl-ring-ref", "%u", &val, NULL);
>> - if (err)
>> + err = xenbus_scanf(XBT_NIL, dev->otherend,
>> + "ctrl-ring-ref", "%u", &val);
>> + if (err <= 0)
>
> Looking at other uses of xenbus_scanf() in the same code I think the check
> here should be if (err < 0). It's a nit, since xenbus_scanf() cannot return 0,
> but it would be better for consistency I think.
Hmm, this goes back to the discussion following from
https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg00678.html
which in fact you had given your R-b back then. I continue to be
of the opinion that callers should not leverage the fact that
xenbus_scanf() can't return zero. They instead should check for
an explicit success indicator (which only positive values are). But
you're the maintainer of the code, so if you now think the same
way David does, I guess I'll have to make the adjustment.
>> goto done; /* The frontend does not have a control ring */
>>
>> ring_ref = val;
>>
>> - err = xenbus_gather(XBT_NIL, dev->otherend,
>> - "event-channel-ctrl", "%u", &val, NULL);
>> - if (err) {
>> + err = xenbus_scanf(XBT_NIL, dev->otherend,
>> + "event-channel-ctrl", "%u", &val);
>> + if (err <= 0) {
>> xenbus_dev_fatal(dev, err,
>> "reading %s/event-channel-ctrl",
>> dev->otherend);
>> @@ -919,7 +919,7 @@ done:
>> return 0;
>>
>> fail:
>> - return err;
>> + return err ?: -ENODATA;
>
> I don't think you need this.
If the other change gets made, then indeed this isn't needed.
Jan
^ permalink raw reply
* Re: [PATCH v3 net] flow_dissector: fix vlan tag handling
From: Jiri Pirko @ 2016-10-25 8:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Eric Garver, Hadar Hen Zion, Jiri Pirko,
Alexander Duyck, Tom Herbert, netdev, linux-kernel
In-Reply-To: <20161024214058.1896091-1-arnd@arndb.de>
Mon, Oct 24, 2016 at 11:40:30PM CEST, arnd@arndb.de wrote:
>gcc warns about an uninitialized pointer dereference in the vlan
>priority handling:
>
>net/core/flow_dissector.c: In function '__skb_flow_dissect':
>net/core/flow_dissector.c:281:61: error: 'vlan' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
>As pointed out by Jiri Pirko, the variable is never actually used
>without being initialized first as the only way it end up uninitialized
>is with skb_vlan_tag_present(skb)==true, and that means it does not
>get accessed.
>
>However, the warning hints at some related issues that I'm addressing
>here:
>
>- the second check for the vlan tag is different from the first one
> that tests the skb for being NULL first, causing both the warning
> and a possible NULL pointer dereference that was not entirely fixed.
>- The same patch that introduced the NULL pointer check dropped an
> earlier optimization that skipped the repeated check of the
> protocol type
>- The local '_vlan' variable is referenced through the 'vlan' pointer
> but the variable has gone out of scope by the time that it is
> accessed, causing undefined behavior
>
>Caching the result of the 'skb && skb_vlan_tag_present(skb)' check
>in a local variable allows the compiler to further optimize the
>later check. With those changes, the warning also disappears.
>
>Fixes: 3805a938a6c2 ("flow_dissector: Check skb for VLAN only if skb specified.")
>Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* RE: [PATCH v2 RESEND] xen-netback: prefer xenbus_scanf() over xenbus_gather()
From: Paul Durrant @ 2016-10-25 7:52 UTC (permalink / raw)
To: Jan Beulich, Wei Liu
Cc: David Vrabel, xen-devel@lists.xenproject.org,
boris.ostrovsky@oracle.com, Juergen Gross, netdev@vger.kernel.org
In-Reply-To: <580E40000200007800119234@prv-mh.provo.novell.com>
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 24 October 2016 16:08
> To: Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu <wei.liu2@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>; xen-devel@lists.xenproject.org;
> boris.ostrovsky@oracle.com; Juergen Gross <JGross@suse.com>;
> netdev@vger.kernel.org
> Subject: [PATCH v2 RESEND] xen-netback: prefer xenbus_scanf() over
> xenbus_gather()
>
> For single items being collected this should be preferred as being more
> typesafe (as the compiler can check format string and to-be-written-to
> variable match) and more efficient (requiring one less parameter to be
> passed).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Avoid commit message to continue from subject.
> ---
> drivers/net/xen-netback/xenbus.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> --- 4.9-rc2/drivers/net/xen-netback/xenbus.c
> +++ 4.9-rc2-xen-netback-prefer-xenbus_scanf/drivers/net/xen-
> netback/xenbus.c
> @@ -889,16 +889,16 @@ static int connect_ctrl_ring(struct back
> unsigned int evtchn;
> int err;
>
> - err = xenbus_gather(XBT_NIL, dev->otherend,
> - "ctrl-ring-ref", "%u", &val, NULL);
> - if (err)
> + err = xenbus_scanf(XBT_NIL, dev->otherend,
> + "ctrl-ring-ref", "%u", &val);
> + if (err <= 0)
Looking at other uses of xenbus_scanf() in the same code I think the check here should be if (err < 0). It's a nit, since xenbus_scanf() cannot return 0, but it would be better for consistency I think.
> goto done; /* The frontend does not have a control ring */
>
> ring_ref = val;
>
> - err = xenbus_gather(XBT_NIL, dev->otherend,
> - "event-channel-ctrl", "%u", &val, NULL);
> - if (err) {
> + err = xenbus_scanf(XBT_NIL, dev->otherend,
> + "event-channel-ctrl", "%u", &val);
> + if (err <= 0) {
> xenbus_dev_fatal(dev, err,
> "reading %s/event-channel-ctrl",
> dev->otherend);
> @@ -919,7 +919,7 @@ done:
> return 0;
>
> fail:
> - return err;
> + return err ?: -ENODATA;
I don't think you need this.
Paul
> }
>
> static void connect(struct backend_info *be)
>
>
^ permalink raw reply
* Re: question about function igmp_stop_timer() in net/ipv4/igmp.c
From: Andrew Lunn @ 2016-10-25 7:39 UTC (permalink / raw)
To: Dongpo Li; +Cc: netdev
In-Reply-To: <580EB1D2.3010309@hisilicon.com>
On Tue, Oct 25, 2016 at 09:13:54AM +0800, Dongpo Li wrote:
> Hi Andrew,
>
> On 2016/10/24 23:32, Andrew Lunn wrote:
> > On Mon, Oct 24, 2016 at 07:50:12PM +0800, Dongpo Li wrote:
> >> Hello
> >>
> >> We encountered a multicast problem when two set-top box(STB) join the same multicast group and leave.
> >> The two boxes can join the same multicast group
> >> but only one box can send the IGMP leave group message when leave,
> >> the other box does not send the IGMP leave message.
> >> Our boxes use the IGMP version 2.
> >>
> >> I added some debug info and found the whole procedure is like this:
> >> (1) Box A joins the multicast group 225.1.101.145 and send the IGMP v2 membership report(join group).
> >> (2) Box B joins the same multicast group 225.1.101.145 and also send the IGMP v2 membership report(join group).
> >> (3) Box A receives the IGMP membership report from Box B and kernel calls igmp_heard_report().
> >> This function will call igmp_stop_timer(im).
> >> In function igmp_stop_timer(im), it tries to delete IGMP timer and does the following:
> >> im->tm_running = 0;
> >> im->reporter = 0;
> >> (4) Box A leaves the multicast group 225.1.101.145 and kernel calls
> >> ip_mc_leave_group -> ip_mc_dec_group -> igmp_group_dropped.
> >> But in function igmp_group_dropped(), the im->reporter is 0, so the kernel does not send the IGMP leave message.
> >
> > RFC 2236 says:
> >
> > 2. Introduction
> >
> > The Internet Group Management Protocol (IGMP) is used by IP hosts to
> > report their multicast group memberships to any immediately-
> > neighboring multicast routers.
> >
> > Are Box A or B multicast routers?
> Thank you for your comments.
> Both Box A and B are IP hosts, not multicast routers.
> And the RFC says: IGMP is used by "IP hosts" to report their multicast group membership.
They report their membership to gateways, not to each other. The
gateway will then arrange for multicast traffic for the group from
other subnets to be forwarded to this subnet. You don't need IGMP to
receive local traffic.
Also, this timer is to do with responding to IGMP querier, typically
the multicast gateway. The querier keeps track of if a group is in use
within a subnet. It listens to group joins and optionally leaves. It
also periodically sends out IGMP querier requests, asking who is
interested in what groups. The hosts use a random delay before
answering, and if some other hosts replies about a group they are a
member of, they don't send a response themselves. One is enough.
Read the RFC.
Andrew
^ permalink raw reply
* Suggestions for new Ethernet driver?
From: David VomLehn @ 2016-10-25 5:10 UTC (permalink / raw)
To: Linux networking mailing list; +Cc: David S. Miller
I'm working with Aquantia to add a new 2.5/5 Gbps driver to the kernel.
It looks like it's going to one of the biggest drivers in
drivers/net/ethernet. The team that developed the driver is new to
kernel development processes, but are working to make it
checkpatch-clean and addressing sparse issues. Right now we're working
to split the code into small chunks for review. The sequence of patches
first targets basic functionality, then adding a feature at a time. Still,
it's going to be a lot of work to review.
Aquantia is committed to doing the work to add this to the mainline
kernel but it's clearly going to be a substantial amount of work not
only for them, but for reviewers. So, my question: what can we do to
make this process easy for the networking community in addition to the
basics that are already under way? I welcome any and all suggestions.
Thanks!
^ permalink raw reply
* [PATCH net-next] ibmveth: calculate correct gso_size and set gso_type
From: Jon Maxwell @ 2016-10-25 5:13 UTC (permalink / raw)
To: tlfalcon
Cc: benh, paulus, mpe, davem, tom, jarod, hofrat, netdev,
linuxppc-dev, linux-kernel, mleitner, jmaxwell, Jon Maxwell
We recently encountered a bug where a few customers using ibmveth on the
same LPAR hit an issue where a TCP session hung when large receive was
enabled. Closer analysis revealed that the session was stuck because the
one side was advertising a zero window repeatedly.
We narrowed this down to the fact the ibmveth driver did not set gso_size
which is translated by TCP into the MSS later up the stack. The MSS is
used to calculate the TCP window size and as that was abnormally large,
it was calculating a zero window, even although the sockets receive buffer
was completely empty.
We were able to reproduce this and worked with IBM to fix this. Thanks Tom
and Marcelo for all your help and review on this.
The patch fixes both our internal reproduction tests and our customers tests.
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
drivers/net/ethernet/ibm/ibmveth.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 29c05d0..3028c33 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1182,6 +1182,8 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
int frames_processed = 0;
unsigned long lpar_rc;
struct iphdr *iph;
+ bool large_packet = 0;
+ u16 hdr_len = ETH_HLEN + sizeof(struct tcphdr);
restart_poll:
while (frames_processed < budget) {
@@ -1236,10 +1238,27 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
iph->check = 0;
iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
adapter->rx_large_packets++;
+ large_packet = 1;
}
}
}
+ if (skb->len > netdev->mtu) {
+ iph = (struct iphdr *)skb->data;
+ if (be16_to_cpu(skb->protocol) == ETH_P_IP && iph->protocol == IPPROTO_TCP) {
+ hdr_len += sizeof(struct iphdr);
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+ skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
+ } else if (be16_to_cpu(skb->protocol) == ETH_P_IPV6 &&
+ iph->protocol == IPPROTO_TCP) {
+ hdr_len += sizeof(struct ipv6hdr);
+ skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+ skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
+ }
+ if (!large_packet)
+ adapter->rx_large_packets++;
+ }
+
napi_gro_receive(napi, skb); /* send it up */
netdev->stats.rx_packets++;
--
1.8.3.1
^ permalink raw reply related
* [PATCH] xfrm: remove unused helper
From: Steffen Klassert @ 2016-10-25 5:05 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1477371941-13336-1-git-send-email-steffen.klassert@secunet.com>
From: Florian Westphal <fw@strlen.de>
Not used anymore since 2009 (9e0d57fd6dad37,
'xfrm: SAD entries do not expire correctly after suspend-resume').
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_state.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 419bf5d..45cb7c6 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -388,14 +388,6 @@ static void xfrm_state_gc_task(struct work_struct *work)
xfrm_state_gc_destroy(x);
}
-static inline unsigned long make_jiffies(long secs)
-{
- if (secs >= (MAX_SCHEDULE_TIMEOUT-1)/HZ)
- return MAX_SCHEDULE_TIMEOUT-1;
- else
- return secs*HZ;
-}
-
static enum hrtimer_restart xfrm_timer_handler(struct hrtimer *me)
{
struct tasklet_hrtimer *thr = container_of(me, struct tasklet_hrtimer, timer);
--
1.9.1
^ permalink raw reply related
* pull request (net-next): ipsec-next 2016-10-25
From: Steffen Klassert @ 2016-10-25 5:05 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
Just a leftover from the last development cycle.
1) Remove some unused code, from Florian Westphal.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit 31fbe81fe3426dfb7f8056a7f5106c6b1841a9aa:
Merge branch 'qcom-emac-acpi' (2016-09-29 01:50:20 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
for you to fetch changes up to 2258d927a691ddd2ab585adb17ea9f96e89d0638:
xfrm: remove unused helper (2016-09-30 08:20:56 +0200)
----------------------------------------------------------------
Florian Westphal (1):
xfrm: remove unused helper
net/xfrm/xfrm_state.c | 8 --------
1 file changed, 8 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next] ethernet: fix min/max MTU typos
From: Jarod Wilson @ 2016-10-25 3:07 UTC (permalink / raw)
To: Stefan Richter; +Cc: David S. Miller, netdev, Thomas Falcon, linux-kernel
In-Reply-To: <20161024144226.70480d4b@kant>
On Mon, Oct 24, 2016 at 02:42:26PM +0200, Stefan Richter wrote:
> Fixes: d894be57ca92('ethernet: use net core MTU range checking in more drivers')
> CC: Jarod Wilson <jarod@redhat.com>
> CC: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Wuf. Thank you, Stefan. Way too many bleeding eyeball hours staring at all
those changes.
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply
* Re: [PATCH net-next 2/2 v2] firewire: net: set initial MTU = 1500 unconditionally, fix IPv6 on some CardBus cards
From: Jarod Wilson @ 2016-10-25 3:05 UTC (permalink / raw)
To: Stefan Richter; +Cc: David S. Miller, netdev, linux1394-devel, linux-kernel
In-Reply-To: <20161024142613.29145f32@kant>
On Mon, Oct 24, 2016 at 02:26:13PM +0200, Stefan Richter wrote:
> firewire-net, like the older eth1394 driver, reduced the initial MTU to
> less than 1500 octets if the local link layer controller's asynchronous
> packet reception limit was lower.
>
> This is bogus, since this reception limit does not have anything to do
> with the transmission limit. Neither did this reduction affect the TX
> path positively, nor could it prevent link fragmentation at the RX path.
>
> Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU
> of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8,
> which would result in an initial MTU of 512 - 16 = 496. On such cards,
> IPv6 could only be employed if the MTU was manually increased to 1280 or
> more, i.e. IPv6 would not work without intervention from userland.
>
> We now always initialize the MTU to 1500, which is the default according
> to RFC 2734 and RFC 3146.
>
> On a VIA VT6316 based CardBus card which was affected by this, changing
> the MTU from 1008 to 1500 also increases TX bandwidth by 6 %.
> RX remains unaffected.
>
> CC: netdev@vger.kernel.org
> CC: linux1394-devel@lists.sourceforge.net
> CC: Jarod Wilson <jarod@redhat.com>
> Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> ---
> v2: use ETH_DATA_LEN, add comment
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply
* [RFC PATCH net-next] net: ethtool: add support for forward error correction modes
From: Vidya Sagar Ravipati @ 2016-10-25 2:50 UTC (permalink / raw)
To: davem, linville, saeedm, galp, netdev, odedw, ariela, Yuval.Mintz,
tzahio
Cc: roees, aviadr, dustin, roopa
From: Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
Forward Error Correction (FEC) modes i.e Base-R
and Reed-Solomon modes are introduced in 25G/40G/100G standards
for providing good BER at high speeds.
Various networking devices which support 25G/40G/100G provides ability
to manage supported FEC modes and the lack of FEC encoding control and
reporting today is a source for itneroperability issues for many vendors.
FEC capability as well as specific FEC mode i.e. Base-R
or RS modes can be requested or advertised through bits D44:47 of base link
codeword.
This patch set intends to provide option under ethtool to manage and report
FEC encoding settings for networking devices as per IEEE 802.3 bj, bm and by
specs.
set-fec/show-fec option(s) are designed to provide control and report
the FEC encoding on the link.
SET FEC option:
root@tor: ethtool --set-fec swp1 encoding [off | RS | BaseR | auto] autoneg [off | on]
Encoding: Types of encoding
Off : Turning off any encoding
RS : enforcing RS-FEC encoding on supported speeds
BaseR : enforcing Base R encoding on supported speeds
Auto : Default FEC settings for divers , and would represent
asking the hardware to essentially go into a best effort mode.
Here are a few examples of what we would expect if encoding=auto:
- if autoneg is on, we are expecting FEC to be negotiated as on or off
as long as protocol supports it
- if the hardware is capable of detecting the FEC encoding on it's
receiver it will reconfigure its encoder to match
- in absence of the above, the configuration would be set to IEEE
defaults.
>From our understanding , this is essentially what most hardware/driver
combinations are doing today in the absence of a way for users to
control the behavior.
SHOW FEC option:
root@tor: ethtool --show-fec swp1
FEC parameters for swp1:
Autonegotiate: off
FEC encodings: RS
ETHTOOL DEVNAME output modification:
ethtool devname output:
root@tor:~# ethtool swp1
Settings for swp1:
root@hpe-7712-03:~# ethtool swp18
Settings for swp18:
Supported ports: [ FIBRE ]
Supported link modes: 40000baseCR4/Full
40000baseSR4/Full
40000baseLR4/Full
100000baseSR4/Full
100000baseCR4/Full
100000baseLR4_ER4/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Supported FEC modes: [RS | BaseR | None | Not reported]
Advertised link modes: Not reported
Advertised pause frame use: No
Advertised auto-negotiation: No
Advertised FEC modes: [RS | BaseR | None | Not reported]
<<<< One or more FEC modes
Speed: 100000Mb/s
Duplex: Full
Port: FIBRE
PHYAD: 106
Transceiver: internal
Auto-negotiation: off
Link detected: yes
This patch includes following changes
a) New ETHTOOL_SFECPARAM/SFECPARAM API, handled by
the new get_fecparam/set_fecparam callbacks, provides support
for configuration of forward error correction modes.
b) Link mode bits for FEC modes i.e. None (No FEC mode), RS, BaseR/FC
are defined so that users can configure these fec modes for supported
and advertising fields as part of link autonegotiation.
Signed-off-by: Vidya Sagar Ravipati <vidya@cumulusnetworks.com>
---
include/linux/ethtool.h | 4 ++++
include/uapi/linux/ethtool.h | 53 ++++++++++++++++++++++++++++++++++++++++++--
net/core/ethtool.c | 34 ++++++++++++++++++++++++++++
3 files changed, 89 insertions(+), 2 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9ded8c6..79a0bab 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -372,5 +372,9 @@ struct ethtool_ops {
struct ethtool_link_ksettings *);
int (*set_link_ksettings)(struct net_device *,
const struct ethtool_link_ksettings *);
+ int (*get_fecparam)(struct net_device *,
+ struct ethtool_fecparam *);
+ int (*set_fecparam)(struct net_device *,
+ struct ethtool_fecparam *);
};
#endif /* _LINUX_ETHTOOL_H */
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 099a420..28ea382 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1224,6 +1224,51 @@ struct ethtool_per_queue_op {
char data[];
};
+/**
+ * struct ethtool_fecparam - Ethernet forward error correction(fec) parameters
+ * @cmd: Command number = %ETHTOOL_GFECPARAM or %ETHTOOL_SFECPARAM
+ * @autoneg: Flag to enable autonegotiation of fec modes(rs,baser)
+ * (D44:47 of base link code word)
+ * @fec: Bitmask of supported FEC modes
+ * @rsvd: Reserved for future extensions. i.e FEC bypass feature.
+ *
+ * Drivers should reject a non-zero setting of @autoneg when
+ * autoneogotiation is disabled (or not supported) for the link.
+ *
+ * If @autoneg is non-zero, the MAC is configured to enable one of
+ * the supported FEC modes according to the result of autonegotiation.
+ * Otherwise, it is configured directly based on the @fec parameter
+ */
+struct ethtool_fecparam {
+ __u32 cmd;
+ __u32 autoneg;
+ /* bitmask of FEC modes */
+ __u32 fec;
+ __u32 reserved;
+};
+
+/**
+ * enum ethtool_fec_config_bits - flags definition of ethtool_fec_configuration
+ * @ETHTOOL_FEC_NONE: FEC mode configuration is not supported
+ * @ETHTOOL_FEC_AUTO: Default/Best FEC mode provided by driver
+ * @ETHTOOL_FEC_OFF: No FEC Mode
+ * @ETHTOOL_FEC_RS: Reed-Solomon Forward Error Detection mode
+ * @ETHTOOL_FEC_BASER: Base-R/Reed-Solomon Forward Error Detection mode
+ */
+enum ethtool_fec_config_bits {
+ ETHTOOL_FEC_NONE_BIT,
+ ETHTOOL_FEC_AUTO_BIT,
+ ETHTOOL_FEC_OFF_BIT,
+ ETHTOOL_FEC_RS_BIT,
+ ETHTOOL_FEC_BASER_BIT,
+};
+
+#define ETHTOOL_FEC_NONE (1 << ETHTOOL_FEC_NONE_BIT)
+#define ETHTOOL_FEC_AUTO (1 << ETHTOOL_FEC_AUTO_BIT)
+#define ETHTOOL_FEC_OFF (1 << ETHTOOL_FEC_OFF_BIT)
+#define ETHTOOL_FEC_RS (1 << ETHTOOL_FEC_RS_BIT)
+#define ETHTOOL_FEC_BASER (1 << ETHTOOL_FEC_BASER_BIT)
+
/* CMDs currently supported */
#define ETHTOOL_GSET 0x00000001 /* DEPRECATED, Get settings.
* Please use ETHTOOL_GLINKSETTINGS
@@ -1315,6 +1360,8 @@ struct ethtool_per_queue_op {
#define ETHTOOL_GLINKSETTINGS 0x0000004c /* Get ethtool_link_settings */
#define ETHTOOL_SLINKSETTINGS 0x0000004d /* Set ethtool_link_settings */
+#define ETHTOOL_GFECPARAM 0x0000004e /* Get FEC settings */
+#define ETHTOOL_SFECPARAM 0x0000004f /* Set FEC settings */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
@@ -1369,7 +1416,9 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_10000baseLR_Full_BIT = 44,
ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT = 45,
ETHTOOL_LINK_MODE_10000baseER_Full_BIT = 46,
-
+ ETHTOOL_LINK_MODE_FEC_NONE_BIT = 47,
+ ETHTOOL_LINK_MODE_FEC_RS_BIT = 48,
+ ETHTOOL_LINK_MODE_FEC_BASER_BIT = 49,
/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
* 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
@@ -1378,7 +1427,7 @@ enum ethtool_link_mode_bit_indices {
*/
__ETHTOOL_LINK_MODE_LAST
- = ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+ = ETHTOOL_LINK_MODE_FEC_BASER_BIT,
};
#define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name) \
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9774898..be8a917 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2422,6 +2422,33 @@ static int ethtool_set_per_queue(struct net_device *dev, void __user *useraddr)
};
}
+static int ethtool_get_fecparam(struct net_device *dev, void __user *useraddr)
+{
+ struct ethtool_fecparam fecparam = { ETHTOOL_GFECPARAM };
+
+ if (!dev->ethtool_ops->get_fecparam)
+ return -EOPNOTSUPP;
+
+ dev->ethtool_ops->get_fecparam(dev, &fecparam);
+
+ if (copy_to_user(useraddr, &fecparam, sizeof(fecparam)))
+ return -EFAULT;
+ return 0;
+}
+
+static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr)
+{
+ struct ethtool_fecparam fecparam;
+
+ if (!dev->ethtool_ops->set_fecparam)
+ return -EOPNOTSUPP;
+
+ if (copy_from_user(&fecparam, useraddr, sizeof(fecparam)))
+ return -EFAULT;
+
+ return dev->ethtool_ops->set_fecparam(dev, &fecparam);
+}
+
/* The main entry point in this file. Called from net/core/dev_ioctl.c */
int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -2479,6 +2506,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_GET_TS_INFO:
case ETHTOOL_GEEE:
case ETHTOOL_GTUNABLE:
+ case ETHTOOL_GFECPARAM:
break;
default:
if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
@@ -2684,6 +2712,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_SLINKSETTINGS:
rc = ethtool_set_link_ksettings(dev, useraddr);
break;
+ case ETHTOOL_GFECPARAM:
+ rc = ethtool_get_fecparam(dev, useraddr);
+ break;
+ case ETHTOOL_SFECPARAM:
+ rc = ethtool_set_fecparam(dev, useraddr);
+ break;
default:
rc = -EOPNOTSUPP;
}
--
2.1.4
^ permalink raw reply related
* Good Day
From: Cashland FInancial @ 2016-10-24 23:40 UTC (permalink / raw)
To: Recipients
Loan Offer at 3%, Feel Free to REPLY back to us for more info
^ permalink raw reply
* RE: [PATCH] net: fec: hard phy reset on open
From: Andy Duan @ 2016-10-25 1:56 UTC (permalink / raw)
To: Manfred Schlaegl; +Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <a1fcd05e-0159-b5cc-8d7b-790145ceb187@ginzinger.com>
From: Manfred Schlaegl <manfred.schlaegl@ginzinger.com> Sent: Monday, October 24, 2016 10:43 PM
> To: Andy Duan <fugang.duan@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] net: fec: hard phy reset on open
>
> On 2016-10-24 16:03, Andy Duan wrote:
> > From: manfred.schlaegl@gmx.at <manfred.schlaegl@gmx.at> Sent:
> Monday,
> > October 24, 2016 5:26 PM
> >> To: Andy Duan <fugang.duan@nxp.com>
> >> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Subject: [PATCH] net: fec: hard phy reset on open
> >>
> >> We have seen some problems with auto negotiation on i.MX6 using
> >> LAN8720, after interface down/up.
> >>
> >> In our configuration, the ptp clock is used externally as reference
> >> clock for the phy. Some phys (e.g. LAN8720) needs a stable clock
> >> while and after hard reset.
> >> Before this patch, the driver disabled the clock on close but did no
> >> hard reset on open, after enabling the clocks again.
> >>
> >> A solution that prevents disabling the clocks on close was
> >> considered, but discarded because of bad power saving behavior.
> >>
> >> This patch saves the reset dt properties on probe and does a reset on
> >> every open after clocks where enabled, to make sure the clock is
> >> stable while and after hard reset.
> >>
> >> Tested on i.MX6 and i.MX28, both using LAN8720.
> >>
> >> Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com>
> >> ---
>
> > This patch did hard reset to let phy stable.
>
> > Firstly, you should do reset before clock enable.
> I have to disagree here.
> The phy demands(datasheet + tests) a stable clock at the time of (hard-
> )reset and after this. Therefore the clock has to be enabled before the hard
> reset.
> (This is exactly the reason for the patch.)
>
> Generally: The sense of a reset is to defer the start of digital circuit until the
> environment (power, clocks, ...) has stabilized.
>
> Furthermore: Before this patch the hard reset was done in fec_probe. And
> here also after the clocks were enabled.
>
> Whats was your argument to do it the other way in this special case?
>
I check some different vendor phy, hard reset assert after clock stable.
But I still don't ensure all phys are this action.
> > Secondly, we suggest to do phy reset in phy driver, not MAC driver.
> I was not sure, if you meant a soft-, or hard-reset here.
>
> In case you are talking about soft reset:
> Yes, the phy drivers perform a soft reset. Sadly a soft reset is not sufficient in
> this case - The phy recovers only on a hard reset from lost clock. (datasheet +
> tests)
>
> In case you're talking about hard reset:
> Intuitively I would also think, that the (hard-)reset should be handled by the
> phy driver, but this is not reality in given implementations.
>
> Documentation/devicetree/bindings/net/fsl-fec.txt says
>
> - phy-reset-gpios : Should specify the gpio for phy reset
>
> It is explicitly talked about phy-reset here. And the (hard-)reset was handled
> by the fec driver also before this patch (once on probe).
>
I suggest to do phy hard reset in phy driver like:
drivers/net/phy/spi_ks8995.c:
and Uwe Kleine-König's patch "phy: add support for a reset-gpio specification" (I don't know why the patch is reverted now.)
Regards,
Andy
> >
> > Regards,
> > Andy
>
> Thanks for your feedback!
>
> Best regards,
> Manfred
>
>
>
> >
> >> drivers/net/ethernet/freescale/fec.h | 4 ++
> >> drivers/net/ethernet/freescale/fec_main.c | 77
> >> +++++++++++++++++-------
> >> -------
> >> 2 files changed, 47 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/freescale/fec.h
> >> b/drivers/net/ethernet/freescale/fec.h
> >> index c865135..379e619 100644
> >> --- a/drivers/net/ethernet/freescale/fec.h
> >> +++ b/drivers/net/ethernet/freescale/fec.h
> >> @@ -498,6 +498,10 @@ struct fec_enet_private {
> >> struct clk *clk_enet_out;
> >> struct clk *clk_ptp;
> >>
> >> + int phy_reset;
> >> + bool phy_reset_active_high;
> >> + int phy_reset_msec;
> >> +
> >> bool ptp_clk_on;
> >> struct mutex ptp_clk_mutex;
> >> unsigned int num_tx_queues;
> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> >> b/drivers/net/ethernet/freescale/fec_main.c
> >> index 48a033e..8cc1ec5 100644
> >> --- a/drivers/net/ethernet/freescale/fec_main.c
> >> +++ b/drivers/net/ethernet/freescale/fec_main.c
> >> @@ -2802,6 +2802,22 @@ static int fec_enet_alloc_buffers(struct
> >> net_device *ndev)
> >> return 0;
> >> }
> >>
> >> +static void fec_reset_phy(struct fec_enet_private *fep) {
> >> + if (!gpio_is_valid(fep->phy_reset))
> >> + return;
> >> +
> >> + gpio_set_value_cansleep(fep->phy_reset, !!fep-
> >>> phy_reset_active_high);
> >> +
> >> + if (fep->phy_reset_msec > 20)
> >> + msleep(fep->phy_reset_msec);
> >> + else
> >> + usleep_range(fep->phy_reset_msec * 1000,
> >> + fep->phy_reset_msec * 1000 + 1000);
> >> +
> >> + gpio_set_value_cansleep(fep->phy_reset, !fep-
> >>> phy_reset_active_high);
> >> +}
> >> +
> >> static int
> >> fec_enet_open(struct net_device *ndev) { @@ -2817,6 +2833,8 @@
> >> fec_enet_open(struct net_device *ndev)
> >> if (ret)
> >> goto clk_enable;
> >>
> >> + fec_reset_phy(fep);
> >> +
> >> /* I should reset the ring buffers here, but I don't yet know
> >> * a simple way to do that.
> >> */
> >> @@ -3183,52 +3201,39 @@ static int fec_enet_init(struct net_device
> *ndev)
> >> return 0;
> >> }
> >>
> >> -#ifdef CONFIG_OF
> >> -static void fec_reset_phy(struct platform_device *pdev)
> >> +static int
> >> +fec_get_reset_phy(struct platform_device *pdev, int *msec, int
> >> *phy_reset,
> >> + bool *active_high)
> >> {
> >> - int err, phy_reset;
> >> - bool active_high = false;
> >> - int msec = 1;
> >> + int err;
> >> struct device_node *np = pdev->dev.of_node;
> >>
> >> - if (!np)
> >> - return;
> >> + if (!np || !of_device_is_available(np))
> >> + return 0;
> >>
> >> - of_property_read_u32(np, "phy-reset-duration", &msec);
> >> + of_property_read_u32(np, "phy-reset-duration", msec);
> >> /* A sane reset duration should not be longer than 1s */
> >> - if (msec > 1000)
> >> - msec = 1;
> >> + if (*msec > 1000)
> >> + *msec = 1;
> >>
> >> - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> >> - if (!gpio_is_valid(phy_reset))
> >> - return;
> >> + *phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> >> + if (!gpio_is_valid(*phy_reset))
> >> + return 0;
> >>
> >> - active_high = of_property_read_bool(np, "phy-reset-active-high");
> >> + *active_high = of_property_read_bool(np, "phy-reset-active-high");
> >>
> >> - err = devm_gpio_request_one(&pdev->dev, phy_reset,
> >> - active_high ? GPIOF_OUT_INIT_HIGH :
> >> GPIOF_OUT_INIT_LOW,
> >> - "phy-reset");
> >> + err = devm_gpio_request_one(&pdev->dev, *phy_reset,
> >> + *active_high ?
> >> + GPIOF_OUT_INIT_HIGH :
> >> + GPIOF_OUT_INIT_LOW,
> >> + "phy-reset");
> >> if (err) {
> >> dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n",
> err);
> >> - return;
> >> + return err;
> >> }
> >>
> >> - if (msec > 20)
> >> - msleep(msec);
> >> - else
> >> - usleep_range(msec * 1000, msec * 1000 + 1000);
> >> -
> >> - gpio_set_value_cansleep(phy_reset, !active_high);
> >> -}
> >> -#else /* CONFIG_OF */
> >> -static void fec_reset_phy(struct platform_device *pdev) -{
> >> - /*
> >> - * In case of platform probe, the reset has been done
> >> - * by machine code.
> >> - */
> >> + return 0;
> >> }
> >> -#endif /* CONFIG_OF */
> >>
> >> static void
> >> fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx,
> >> int
> >> *num_rx) @@ -3409,7 +3414,10 @@ fec_probe(struct platform_device
> >> *pdev)
> >> pm_runtime_set_active(&pdev->dev);
> >> pm_runtime_enable(&pdev->dev);
> >>
> >> - fec_reset_phy(pdev);
> >> + ret = fec_get_reset_phy(pdev, &fep->phy_reset_msec, &fep-
> >>> phy_reset,
> >> + &fep->phy_reset_active_high);
> >> + if (ret)
> >> + goto failed_reset;
> >>
> >> if (fep->bufdesc_ex)
> >> fec_ptp_init(pdev);
> >> @@ -3467,6 +3475,7 @@ fec_probe(struct platform_device *pdev)
> >> failed_mii_init:
> >> failed_irq:
> >> failed_init:
> >> +failed_reset:
> >> fec_ptp_stop(pdev);
> >> if (fep->reg_phy)
> >> regulator_disable(fep->reg_phy);
> >> --
> >> 2.1.4
>
^ permalink raw reply
* [PATCH v2] net: skip genenerating uevents for network namespaces that are exiting
From: Andrei Vagin @ 2016-10-25 2:09 UTC (permalink / raw)
To: David S. Miller
Cc: Eric W . Biederman, containers, linux-kernel, netdev,
Andrei Vagin, Cong Wang
No one can see these events, because a network namespace can not be
destroyed, if it has sockets.
Unlike other devices, uevent-s for network devices are generated
only inside their network namespaces. They are filtered in
kobj_bcast_filter()
My experiments shows that net namespaces are destroyed more 30% faster
with this optimization.
Here is a perf output for destroying network namespaces without this
patch.
- 94.76% 0.02% kworker/u48:1 [kernel.kallsyms] [k] cleanup_net
- 94.74% cleanup_net
- 94.64% ops_exit_list.isra.4
- 41.61% default_device_exit_batch
- 41.47% unregister_netdevice_many
- rollback_registered_many
- 40.36% netdev_unregister_kobject
- 14.55% device_del
+ 13.71% kobject_uevent
- 13.04% netdev_queue_update_kobjects
+ 12.96% kobject_put
- 12.72% net_rx_queue_update_kobjects
kobject_put
- kobject_release
+ 12.69% kobject_uevent
+ 0.80% call_netdevice_notifiers_info
+ 19.57% nfsd_exit_net
+ 11.15% tcp_net_metrics_exit
+ 8.25% rpcsec_gss_exit_net
It's very critical to optimize the exit path for network namespaces,
because they are destroyed under net_mutex and many namespaces can be
destroyed for one iteration.
v2: use dev_set_uevent_suppress()
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Andrei Vagin <avagin@openvz.org>
---
net/core/net-sysfs.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 6e4f347..d4fe286 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -950,10 +950,13 @@ net_rx_queue_update_kobjects(struct net_device *dev, int old_num, int new_num)
}
while (--i >= new_num) {
+ struct kobject *kobj = &dev->_rx[i].kobj;
+
+ if (!list_empty(&dev_net(dev)->exit_list))
+ kobj->uevent_suppress = 1;
if (dev->sysfs_rx_queue_group)
- sysfs_remove_group(&dev->_rx[i].kobj,
- dev->sysfs_rx_queue_group);
- kobject_put(&dev->_rx[i].kobj);
+ sysfs_remove_group(kobj, dev->sysfs_rx_queue_group);
+ kobject_put(kobj);
}
return error;
@@ -1340,6 +1343,8 @@ netdev_queue_update_kobjects(struct net_device *dev, int old_num, int new_num)
while (--i >= new_num) {
struct netdev_queue *queue = dev->_tx + i;
+ if (!list_empty(&dev_net(dev)->exit_list))
+ queue->kobj.uevent_suppress = 1;
#ifdef CONFIG_BQL
sysfs_remove_group(&queue->kobj, &dql_group);
#endif
@@ -1525,6 +1530,9 @@ void netdev_unregister_kobject(struct net_device *ndev)
{
struct device *dev = &(ndev->dev);
+ if (!list_empty(&dev_net(ndev)->exit_list))
+ dev_set_uevent_suppress(dev, 1);
+
kobject_get(&dev->kobj);
remove_queue_kobjects(ndev);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next] net: add an ioctl to get a socket network namespace
From: Andrei Vagin @ 2016-10-25 1:29 UTC (permalink / raw)
To: Eric W . Biederman, David S. Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrey Vagin,
linux-api-u79uwXL29TY76Z2rM5mHXA
From: Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
Each socket operates in a network namespace where it has been created,
so if we want to dump and restore a socket, we have to know its network
namespace.
We have a socket_diag to get information about sockets, it doesn't
report sockets which are not bound or connected.
This patch introduces a new socket ioctl, which is called SIOCGSKNS
and used to get a file descriptor for a socket network namespace.
A task must have CAP_NET_ADMIN in a target network namespace to
use this ioctl.
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Andrei Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
---
fs/nsfs.c | 2 +-
include/linux/proc_fs.h | 4 ++++
include/uapi/linux/sockios.h | 1 +
net/socket.c | 13 +++++++++++++
4 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/fs/nsfs.c b/fs/nsfs.c
index 8718af8..8c9fb29 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -118,7 +118,7 @@ void *ns_get_path(struct path *path, struct task_struct *task,
return ret;
}
-static int open_related_ns(struct ns_common *ns,
+int open_related_ns(struct ns_common *ns,
struct ns_common *(*get_ns)(struct ns_common *ns))
{
struct path path = {};
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index b97bf2e..368c7ad 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -82,4 +82,8 @@ static inline struct proc_dir_entry *proc_net_mkdir(
return proc_mkdir_data(name, 0, parent, net);
}
+struct ns_common;
+int open_related_ns(struct ns_common *ns,
+ struct ns_common *(*get_ns)(struct ns_common *ns));
+
#endif /* _LINUX_PROC_FS_H */
diff --git a/include/uapi/linux/sockios.h b/include/uapi/linux/sockios.h
index 8e7890b..83cc54c 100644
--- a/include/uapi/linux/sockios.h
+++ b/include/uapi/linux/sockios.h
@@ -84,6 +84,7 @@
#define SIOCWANDEV 0x894A /* get/set netdev parameters */
#define SIOCOUTQNSD 0x894B /* output queue size (not sent only) */
+#define SIOCGSKNS 0x894C /* get socket network namespace */
/* ARP cache control calls. */
/* 0x8950 - 0x8952 * obsolete calls, don't re-use */
diff --git a/net/socket.c b/net/socket.c
index 5a9bf5e..970a7ea 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -877,6 +877,11 @@ static long sock_do_ioctl(struct net *net, struct socket *sock,
* what to do with it - that's up to the protocol still.
*/
+static struct ns_common *get_net_ns(struct ns_common *ns)
+{
+ return &get_net(container_of(ns, struct net, ns))->ns;
+}
+
static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
{
struct socket *sock;
@@ -945,6 +950,13 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
err = dlci_ioctl_hook(cmd, argp);
mutex_unlock(&dlci_ioctl_mutex);
break;
+ case SIOCGSKNS:
+ err = -EPERM;
+ if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+ break;
+
+ err = open_related_ns(&net->ns, get_net_ns);
+ break;
default:
err = sock_do_ioctl(net, sock, cmd, arg);
break;
@@ -3093,6 +3105,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
case SIOCSIFVLAN:
case SIOCADDDLCI:
case SIOCDELDLCI:
+ case SIOCGSKNS:
return sock_ioctl(file, cmd, arg);
case SIOCGIFFLAGS:
--
2.7.4
^ permalink raw reply related
* Re: [net-next PATCH RFC 02/26] swiotlb: Add support for DMA_ATTR_SKIP_CPU_SYNC
From: Konrad Rzeszutek Wilk @ 2016-10-25 1:22 UTC (permalink / raw)
To: Alexander Duyck
Cc: Konrad Rzeszutek Wilk, Alexander Duyck, Netdev,
linux-kernel@vger.kernel.org, linux-mm, Jesper Dangaard Brouer,
David Miller
In-Reply-To: <CAKgT0UfTSmWGBqE0uDG40sAm-LVwCJ6zM1AFJ8o_tWu+XJvfVw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 342 bytes --]
>
>
> >
> > This too. Why can't that be part of the existing code that was there?
>
> Once again it was a formatting thing. I was indented too far and
> adding the attribute pushed me over 80 characters so I broke it out to
> a label to avoid the problem.
>
Aah. It is OK to go over the 80 characters. I am not that nit picky.
> - Alex
>
[-- Attachment #2: Type: text/html, Size: 768 bytes --]
^ permalink raw reply
* Re: question about function igmp_stop_timer() in net/ipv4/igmp.c
From: Dongpo Li @ 2016-10-25 1:13 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20161024153200.GM1110@lunn.ch>
Hi Andrew,
On 2016/10/24 23:32, Andrew Lunn wrote:
> On Mon, Oct 24, 2016 at 07:50:12PM +0800, Dongpo Li wrote:
>> Hello
>>
>> We encountered a multicast problem when two set-top box(STB) join the same multicast group and leave.
>> The two boxes can join the same multicast group
>> but only one box can send the IGMP leave group message when leave,
>> the other box does not send the IGMP leave message.
>> Our boxes use the IGMP version 2.
>>
>> I added some debug info and found the whole procedure is like this:
>> (1) Box A joins the multicast group 225.1.101.145 and send the IGMP v2 membership report(join group).
>> (2) Box B joins the same multicast group 225.1.101.145 and also send the IGMP v2 membership report(join group).
>> (3) Box A receives the IGMP membership report from Box B and kernel calls igmp_heard_report().
>> This function will call igmp_stop_timer(im).
>> In function igmp_stop_timer(im), it tries to delete IGMP timer and does the following:
>> im->tm_running = 0;
>> im->reporter = 0;
>> (4) Box A leaves the multicast group 225.1.101.145 and kernel calls
>> ip_mc_leave_group -> ip_mc_dec_group -> igmp_group_dropped.
>> But in function igmp_group_dropped(), the im->reporter is 0, so the kernel does not send the IGMP leave message.
>
> RFC 2236 says:
>
> 2. Introduction
>
> The Internet Group Management Protocol (IGMP) is used by IP hosts to
> report their multicast group memberships to any immediately-
> neighboring multicast routers.
>
> Are Box A or B multicast routers?
Thank you for your comments.
Both Box A and B are IP hosts, not multicast routers.
And the RFC says: IGMP is used by "IP hosts" to report their multicast group membership.
>
> Andrew
>
> .
>
Regards,
Dongpo
.
^ permalink raw reply
* Re: [PATCH net-next RFC WIP] Patch for XDP support for virtio_net
From: Alexei Starovoitov @ 2016-10-25 1:10 UTC (permalink / raw)
To: Shrijeet Mukherjee
Cc: Stephen Hemminger, Shrijeet Mukherjee, mst, Tom Herbert, Netdev,
Roopa Prabhu, Nikolay Aleksandrov
In-Reply-To: <CAJmoNQFJhvc4VSdTRWcTZD7T2E1LQ_2BZoMQhZM18jFH8hpQCQ@mail.gmail.com>
On Sun, Oct 23, 2016 at 06:51:53PM -0700, Shrijeet Mukherjee wrote:
>
> The main goal of this patch was to start that discussion. My v2 patch
> rejects the ndo op if neither of rx_mergeable or big_buffers are set.
> Does that sound like a good tradeoff ? Don't know enough about who
> turns these features off and why.
>
> I can say that virtualbox always has the device features enabled .. so
> seems like a good tradeoff ?
If virtio can be taught to work with xdp that would be awesome.
I've looked at it from xdp prog debugging point of view, but amount of
complexity related to mergeable/big/etc was too much, so I went with e1k+xdp.
Are you sure that if mergeable/big disabled than buf is contiguous?
Also my understanding that buf is not writeable?
I don't see how to do TX either... May be it's all solvable somehow.
There was a discussion to convert raw dma buffer in mlx/intel directly
into vhost to avoid skb. This will allow host to send packets into
VMs quickly. Then if we can have fast virtio in the guest then
even more interesting use cases will be solved.
^ permalink raw reply
* [PATCH v2 net 1/1] net sched filters: fix notification of filter delete with proper handle
From: Jamal Hadi Salim @ 2016-10-25 0:18 UTC (permalink / raw)
To: davem; +Cc: netdev, daniel, xiyou.wangcong, eric.dumazet, jiri,
Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Daniel says:
While trying out [1][2], I noticed that tc monitor doesn't show the
correct handle on delete:
$ tc monitor
qdisc clsact ffff: dev eno1 parent ffff:fff1
filter dev eno1 ingress protocol all pref 49152 bpf handle 0x2a [...]
deleted filter dev eno1 ingress protocol all pref 49152 bpf handle 0xf3be0c80
some context to explain the above:
The user identity of any tc filter is represented by a 32-bit
identifier encoded in tcm->tcm_handle. Example 0x2a in the bpf filter
above. A user wishing to delete, get or even modify a specific filter
uses this handle to reference it.
Every classifier is free to provide its own semantics for the 32 bit handle.
Example: classifiers like u32 use schemes like 800:1:801 to describe
the semantics of their filters represented as hash table, bucket and
node ids etc.
Classifiers also have internal per-filter representation which is different
from this externally visible identity. Most classifiers set this
internal representation to be a pointer address (which allows fast retrieval
of said filters in their implementations). This internal representation
is referenced with the "fh" variable in the kernel control code.
When a user successfuly deletes a specific filter, by specifying the correct
tcm->tcm_handle, an event is generated to user space which indicates
which specific filter was deleted.
Before this patch, the "fh" value was sent to user space as the identity.
As an example what is shown in the sample bpf filter delete event above
is 0xf3be0c80. This is infact a 32-bit truncation of 0xffff8807f3be0c80
which happens to be a 64-bit memory address of the internal filter
representation (address of the corresponding filter's struct cls_bpf_prog);
After this patch the appropriate user identifiable handle as encoded
in the originating request tcm->tcm_handle is generated in the event.
One of the cardinal rules of netlink rules is to be able to take an
event (such as a delete in this case) and reflect it back to the
kernel and successfully delete the filter. This patch achieves that.
Note, this issue has existed since the original TC action
infrastructure code patch back in 2004 as found in:
https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/
[1] http://patchwork.ozlabs.org/patch/682828/
[2] http://patchwork.ozlabs.org/patch/682829/
Fixes: 4e54c4816bfe ("[NET]: Add tc extensions infrastructure.")
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_api.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2ee29a3..2b2a797 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -345,7 +345,8 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n)
if (err == 0) {
struct tcf_proto *next = rtnl_dereference(tp->next);
- tfilter_notify(net, skb, n, tp, fh,
+ tfilter_notify(net, skb, n, tp,
+ t->tcm_handle,
RTM_DELTFILTER, false);
if (tcf_destroy(tp, false))
RCU_INIT_POINTER(*back, next);
--
1.9.1
^ permalink raw reply related
* RE
From: Daniel William @ 2016-10-24 22:29 UTC (permalink / raw)
Hello.
Do you need a loan to pay off your dept or loan to buy a house or loan
to start up a business? have you been denied by any bank or loan
company,worry no more,we give loan with 2% interest rate,with a
repayment
plans of 1 to 35 years.Fill the below loan application form and get
your loan contact email sunloan613@gmail.com
The following details are required from you as the loan Applicant.
INFORMATION REQUIRED
PERSONAL DETAIL
Full name:
Age
Phone number:
Marital Status:
Sex
Occupation :
Contact Address:
Country of residence:
Alternate Email:
LOAN DETAIL
Loan amount:
Duration:
Monthly Income:
Have you applied for loans online in the past
Purpose of loan
Note: Send a scanned copy of either your Driver license or Working ID
card will be required for loan approval
We hope to provide our service to you.
Regards
Thanks and GOD BLESS
^ permalink raw reply
* [PATCH] net: bgmac: fix spelling mistake: "connecton" -> "connection"
From: Colin King @ 2016-10-24 22:46 UTC (permalink / raw)
To: David S . Miller, Florian Fainelli, Jon Mason, Arnd Bergmann,
Rafał Miłecki, Philippe Reynes, netdev
Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
trivial fix to spelling mistake in dev_err message
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/broadcom/bgmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 856379c..31ca204 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1449,7 +1449,7 @@ static int bgmac_phy_connect(struct bgmac *bgmac)
phy_dev = phy_connect(bgmac->net_dev, bus_id, &bgmac_adjust_link,
PHY_INTERFACE_MODE_MII);
if (IS_ERR(phy_dev)) {
- dev_err(bgmac->dev, "PHY connecton failed\n");
+ dev_err(bgmac->dev, "PHY connection failed\n");
return PTR_ERR(phy_dev);
}
--
2.9.3
^ permalink raw reply related
* [PATCH v3 net] flow_dissector: fix vlan tag handling
From: Arnd Bergmann @ 2016-10-24 21:40 UTC (permalink / raw)
To: David S. Miller
Cc: Eric Garver, Hadar Hen Zion, Jiri Pirko, Arnd Bergmann,
Alexander Duyck, Tom Herbert, netdev, linux-kernel
gcc warns about an uninitialized pointer dereference in the vlan
priority handling:
net/core/flow_dissector.c: In function '__skb_flow_dissect':
net/core/flow_dissector.c:281:61: error: 'vlan' may be used uninitialized in this function [-Werror=maybe-uninitialized]
As pointed out by Jiri Pirko, the variable is never actually used
without being initialized first as the only way it end up uninitialized
is with skb_vlan_tag_present(skb)==true, and that means it does not
get accessed.
However, the warning hints at some related issues that I'm addressing
here:
- the second check for the vlan tag is different from the first one
that tests the skb for being NULL first, causing both the warning
and a possible NULL pointer dereference that was not entirely fixed.
- The same patch that introduced the NULL pointer check dropped an
earlier optimization that skipped the repeated check of the
protocol type
- The local '_vlan' variable is referenced through the 'vlan' pointer
but the variable has gone out of scope by the time that it is
accessed, causing undefined behavior
Caching the result of the 'skb && skb_vlan_tag_present(skb)' check
in a local variable allows the compiler to further optimize the
later check. With those changes, the warning also disappears.
Fixes: 3805a938a6c2 ("flow_dissector: Check skb for VLAN only if skb specified.")
Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v3: set 'proto' variable correct again
mark it for net, rather than net-next, as both patches that
introduced the bugs are in mainline or in net/master
v2: fix multiple issues found in the initial review beyond the
uninitialized access that turned out to be ok
net/core/flow_dissector.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 44e6ba9d3a6b..ab193e5def07 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -246,13 +246,13 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
case htons(ETH_P_8021AD):
case htons(ETH_P_8021Q): {
const struct vlan_hdr *vlan;
+ struct vlan_hdr _vlan;
+ bool vlan_tag_present = skb && skb_vlan_tag_present(skb);
- if (skb && skb_vlan_tag_present(skb))
+ if (vlan_tag_present)
proto = skb->protocol;
- if (eth_type_vlan(proto)) {
- struct vlan_hdr _vlan;
-
+ if (!vlan_tag_present || eth_type_vlan(skb->protocol)) {
vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
data, hlen, &_vlan);
if (!vlan)
@@ -270,7 +270,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
FLOW_DISSECTOR_KEY_VLAN,
target_container);
- if (skb_vlan_tag_present(skb)) {
+ if (vlan_tag_present) {
key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
key_vlan->vlan_priority =
(skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] netfilter: ip_vs_sync: fix bogus maybe-uninitialized warning
From: Arnd Bergmann @ 2016-10-24 20:21 UTC (permalink / raw)
To: Julian Anastasov
Cc: Wensong Zhang, Simon Horman, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, Quentin Armitage, netdev,
lvs-devel, netfilter-devel, coreteam, linux-kernel
In-Reply-To: <alpine.LFD.2.11.1610242226310.2276@ja.home.ssi.bg>
On Monday, October 24, 2016 10:47:54 PM CEST Julian Anastasov wrote:
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> > index 1b07578bedf3..9350530c16c1 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> > @@ -283,6 +283,7 @@ struct ip_vs_sync_buff {
> > */
> > static void ntoh_seq(struct ip_vs_seq *no, struct ip_vs_seq *ho)
> > {
> > + memset(ho, 0, sizeof(*ho));
> > ho->init_seq = get_unaligned_be32(&no->init_seq);
> > ho->delta = get_unaligned_be32(&no->delta);
> > ho->previous_delta = get_unaligned_be32(&no->previous_delta);
>
> So, now there is a double write here?
Correct. I would hope that a sane version of gcc would just not
perform the first write. What happens instead is that the version
that produces the warning here moves the initialization to the
top of the calling function.
> What about such constructs?:
>
> *ho = (struct ip_vs_seq) {
> .init_seq = get_unaligned_be32(&no->init_seq),
> ...
> };
>
> Any difference in the compiled code or warnings?
Yes, it's one of many things I tried. What happens here is that
the warning remains as long as all fields are initialized together,
e.g. these two produces the same warning:
a)
ho->init_seq = get_unaligned_be32(&no->init_seq);
ho->delta = get_unaligned_be32(&no->delta);
ho->previous_delta = get_unaligned_be32(&no->previous_delta);
b)
*ho = (struct ip_vs_seq) {
.init_seq = get_unaligned_be32(&no->init_seq);
.delta = get_unaligned_be32(&no->delta);
.previous_delta = get_unaligned_be32(&no->previous_delta);
};
but this one does not:
c)
*ho = (struct ip_vs_seq) {
.delta = get_unaligned_be32(&no->delta);
.previous_delta = get_unaligned_be32(&no->previous_delta);
};
ho->init_seq = get_unaligned_be32(&no->init_seq);
I have absolutely no idea what is going on inside of gcc here.
Arnd
^ permalink raw reply
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
From: Cong Wang @ 2016-10-24 20:17 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Andrey Konovalov, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
In-Reply-To: <CAM_iQpXj3ktALCmjwaPKJuf0SDEc6Txk-yYkRzZ79LFEsxOx3w@mail.gmail.com>
On Mon, Oct 24, 2016 at 1:10 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Oct 24, 2016 at 12:11 PM, Oliver Hartkopp
> <socketcan@hartkopp.net> wrote:
>> if (proc_dir) {
>> /* unique socket address as filename */
>> sprintf(bo->procname, "%lu", sock_i_ino(sk));
>> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
>> proc_dir,
>> &bcm_proc_fops, sk);
>> + if (!bo->bcm_proc_read) {
>> + ret = -ENOMEM;
>> + goto fail;
>> + }
>
> Well, I meant we need to call proc_create_data() once per socket,
> so we need a check before proc_create_data() too.
Hmm, bo->bound should guarantee it, so never mind, your patch
looks fine.
^ permalink raw reply
* Re: [PATCH] can: fix warning in bcm_connect/proc_register
From: Cong Wang @ 2016-10-24 20:10 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Andrey Konovalov, David Miller, linux-can,
Linux Kernel Network Developers, LKML, syzkaller,
Kostya Serebryany, Alexander Potapenko, Dmitry Vyukov,
Eric Dumazet
In-Reply-To: <20161024191126.30256-1-socketcan@hartkopp.net>
On Mon, Oct 24, 2016 at 12:11 PM, Oliver Hartkopp
<socketcan@hartkopp.net> wrote:
> if (proc_dir) {
> /* unique socket address as filename */
> sprintf(bo->procname, "%lu", sock_i_ino(sk));
> bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
> proc_dir,
> &bcm_proc_fops, sk);
> + if (!bo->bcm_proc_read) {
> + ret = -ENOMEM;
> + goto fail;
> + }
Well, I meant we need to call proc_create_data() once per socket,
so we need a check before proc_create_data() too.
Thanks.
^ 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