* Re: [PATCH V2 1/1] net: cdc_ncm: Fix TX zero padding
From: David Miller @ 2017-05-08 20:01 UTC (permalink / raw)
To: jim_baxter; +Cc: linux-usb, netdev, linux-kernel, oliver
In-Reply-To: <1494247797-1732-2-git-send-email-jim_baxter@mentor.com>
From: Jim Baxter <jim_baxter@mentor.com>
Date: Mon, 8 May 2017 13:49:57 +0100
> The zero padding that is added to NTB's does
> not zero the memory correctly.
> This is because the skb_put modifies the value
> of skb_out->len which results in the memset
> command not setting any memory to zero as
> (ctx->tx_max - skb_out->len) == 0.
>
> I have resolved this by storing the size of
> the memory to be zeroed before the skb_put
> and using this in the memset call.
>
> Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
> Reviewed-by: Bjørn Mork <bjorn@mork.no>
Applied, thank you.
^ permalink raw reply
* Re: pull-request: mac80211 2017-05-08
From: David Miller @ 2017-05-08 20:08 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20170508144622.19986-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 8 May 2017 16:46:21 +0200
> Thanks for merging the rate fix quickly the other day. I've got a
> few more fixes lined up, so this time as a pull request.
>
> Please pull and let me know if there's any problem.
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH RFC net-next 3/6] net: Introduce IFF_LWT_NETDEV flag
From: David Miller @ 2017-05-08 20:11 UTC (permalink / raw)
To: johannes; +Cc: dsahern, netdev, roopa, f.fainelli, nicolas.dichtel
In-Reply-To: <1494233712.2562.0.camel@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 08 May 2017 10:55:12 +0200
>
>> +static inline bool netif_is_lwd(struct net_device *dev)
>> +{
>> + return !!(dev->priv_flags & IFF_LWT_NETDEV);
>> +}
>
> Am I the only one who thinks that this "LWT_NETDEV" vs "LWD" is a bit
> confusing?
Agreed, my old eyes can't discern them at a distance :-)
^ permalink raw reply
* [PATCH net-next] geneve: add rtnl changelink support
From: Girish Moodalbail @ 2017-05-08 20:08 UTC (permalink / raw)
To: netdev; +Cc: davem, pshelar, joe, jbenc
This patch adds changelink rtnl operation support for geneve devices.
Code changes involve:
- refactor geneve_newlink into geneve_nl2info to be used by both
geneve_newlink and geneve_changelink
- geneve_nl2info takes a changelink boolean argument to isolate
changelink checks and updates.
- Allow changing only a few attributes:
- return -EOPNOTSUPP for attributes that cannot be changed for
now. Incremental patches can make the non-supported one
available in the future if needed.
Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
---
drivers/net/geneve.c | 149 ++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 117 insertions(+), 32 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index dec5d56..6528910 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1112,6 +1112,18 @@ static bool is_tnl_info_zero(const struct ip_tunnel_info *info)
return true;
}
+static inline bool geneve_dst_addr_equal(struct ip_tunnel_info *a,
+ struct ip_tunnel_info *b)
+{
+ if (ip_tunnel_info_af(a) != ip_tunnel_info_af(b))
+ return false;
+
+ if (ip_tunnel_info_af(a) == AF_INET)
+ return a->key.u.ipv4.dst == b->key.u.ipv4.dst;
+ else
+ return ipv6_addr_equal(&a->key.u.ipv6.dst, &b->key.u.ipv6.dst);
+}
+
static int geneve_configure(struct net *net, struct net_device *dev,
const struct ip_tunnel_info *info,
bool metadata, bool ipv6_rx_csum)
@@ -1169,45 +1181,58 @@ static void init_tnl_info(struct ip_tunnel_info *info, __u16 dst_port)
info->key.tp_dst = htons(dst_port);
}
-static int geneve_newlink(struct net *net, struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
+static int geneve_nl2info(struct net_device *dev, struct nlattr *tb[],
+ struct nlattr *data[], struct ip_tunnel_info *info,
+ bool *metadata, bool *use_udp6_rx_checksums,
+ bool changelink)
{
- bool use_udp6_rx_checksums = false;
- struct ip_tunnel_info info;
- bool metadata = false;
+ struct geneve_dev *geneve = netdev_priv(dev);
- init_tnl_info(&info, GENEVE_UDP_PORT);
+ if (changelink) {
+ /* if changelink operation, start with old existing info */
+ memcpy(info, &geneve->info, sizeof(*info));
+ *metadata = geneve->collect_md;
+ *use_udp6_rx_checksums = geneve->use_udp6_rx_checksums;
+ } else {
+ init_tnl_info(info, GENEVE_UDP_PORT);
+ }
if (data[IFLA_GENEVE_REMOTE] && data[IFLA_GENEVE_REMOTE6])
return -EINVAL;
if (data[IFLA_GENEVE_REMOTE]) {
- info.key.u.ipv4.dst =
+ info->key.u.ipv4.dst =
nla_get_in_addr(data[IFLA_GENEVE_REMOTE]);
- if (IN_MULTICAST(ntohl(info.key.u.ipv4.dst))) {
+ if (IN_MULTICAST(ntohl(info->key.u.ipv4.dst))) {
netdev_dbg(dev, "multicast remote is unsupported\n");
return -EINVAL;
}
+ if (changelink &&
+ ip_tunnel_info_af(&geneve->info) == AF_INET6) {
+ info->mode &= ~IP_TUNNEL_INFO_IPV6;
+ info->key.tun_flags &= ~TUNNEL_CSUM;
+ *use_udp6_rx_checksums = false;
+ }
}
if (data[IFLA_GENEVE_REMOTE6]) {
#if IS_ENABLED(CONFIG_IPV6)
- info.mode = IP_TUNNEL_INFO_IPV6;
- info.key.u.ipv6.dst =
+ info->mode = IP_TUNNEL_INFO_IPV6;
+ info->key.u.ipv6.dst =
nla_get_in6_addr(data[IFLA_GENEVE_REMOTE6]);
- if (ipv6_addr_type(&info.key.u.ipv6.dst) &
+ if (ipv6_addr_type(&info->key.u.ipv6.dst) &
IPV6_ADDR_LINKLOCAL) {
netdev_dbg(dev, "link-local remote is unsupported\n");
return -EINVAL;
}
- if (ipv6_addr_is_multicast(&info.key.u.ipv6.dst)) {
+ if (ipv6_addr_is_multicast(&info->key.u.ipv6.dst)) {
netdev_dbg(dev, "multicast remote is unsupported\n");
return -EINVAL;
}
- info.key.tun_flags |= TUNNEL_CSUM;
- use_udp6_rx_checksums = true;
+ info->key.tun_flags |= TUNNEL_CSUM;
+ *use_udp6_rx_checksums = true;
#else
return -EPFNOSUPPORT;
#endif
@@ -1216,48 +1241,107 @@ static int geneve_newlink(struct net *net, struct net_device *dev,
if (data[IFLA_GENEVE_ID]) {
__u32 vni;
__u8 tvni[3];
+ __be64 tunid;
vni = nla_get_u32(data[IFLA_GENEVE_ID]);
tvni[0] = (vni & 0x00ff0000) >> 16;
tvni[1] = (vni & 0x0000ff00) >> 8;
tvni[2] = vni & 0x000000ff;
- info.key.tun_id = vni_to_tunnel_id(tvni);
+ tunid = vni_to_tunnel_id(tvni);
+ if (changelink && (tunid != info->key.tun_id))
+ return -EOPNOTSUPP;
+ info->key.tun_id = tunid;
}
+
if (data[IFLA_GENEVE_TTL])
- info.key.ttl = nla_get_u8(data[IFLA_GENEVE_TTL]);
+ info->key.ttl = nla_get_u8(data[IFLA_GENEVE_TTL]);
if (data[IFLA_GENEVE_TOS])
- info.key.tos = nla_get_u8(data[IFLA_GENEVE_TOS]);
+ info->key.tos = nla_get_u8(data[IFLA_GENEVE_TOS]);
if (data[IFLA_GENEVE_LABEL]) {
- info.key.label = nla_get_be32(data[IFLA_GENEVE_LABEL]) &
+ info->key.label = nla_get_be32(data[IFLA_GENEVE_LABEL]) &
IPV6_FLOWLABEL_MASK;
- if (info.key.label && (!(info.mode & IP_TUNNEL_INFO_IPV6)))
+ if (info->key.label && (!(info->mode & IP_TUNNEL_INFO_IPV6)))
return -EINVAL;
}
- if (data[IFLA_GENEVE_PORT])
- info.key.tp_dst = nla_get_be16(data[IFLA_GENEVE_PORT]);
+ if (data[IFLA_GENEVE_PORT]) {
+ if (changelink)
+ return -EOPNOTSUPP;
+ info->key.tp_dst = nla_get_be16(data[IFLA_GENEVE_PORT]);
+ }
+
+ if (data[IFLA_GENEVE_COLLECT_METADATA]) {
+ if (changelink)
+ return -EOPNOTSUPP;
+ *metadata = true;
+ }
+
+ if (data[IFLA_GENEVE_UDP_CSUM]) {
+ if (changelink)
+ return -EOPNOTSUPP;
+ if (nla_get_u8(data[IFLA_GENEVE_UDP_CSUM]))
+ info->key.tun_flags |= TUNNEL_CSUM;
+ }
+
+ if (data[IFLA_GENEVE_UDP_ZERO_CSUM6_TX]) {
+ if (changelink)
+ return -EOPNOTSUPP;
+ if (nla_get_u8(data[IFLA_GENEVE_UDP_ZERO_CSUM6_TX]))
+ info->key.tun_flags &= ~TUNNEL_CSUM;
+ }
- if (data[IFLA_GENEVE_COLLECT_METADATA])
- metadata = true;
+ if (data[IFLA_GENEVE_UDP_ZERO_CSUM6_RX]) {
+ if (changelink)
+ return -EOPNOTSUPP;
+ if (nla_get_u8(data[IFLA_GENEVE_UDP_ZERO_CSUM6_RX]))
+ *use_udp6_rx_checksums = false;
+ }
- if (data[IFLA_GENEVE_UDP_CSUM] &&
- nla_get_u8(data[IFLA_GENEVE_UDP_CSUM]))
- info.key.tun_flags |= TUNNEL_CSUM;
+ return 0;
+}
- if (data[IFLA_GENEVE_UDP_ZERO_CSUM6_TX] &&
- nla_get_u8(data[IFLA_GENEVE_UDP_ZERO_CSUM6_TX]))
- info.key.tun_flags &= ~TUNNEL_CSUM;
+static int geneve_newlink(struct net *net, struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[])
+{
+ bool use_udp6_rx_checksums = false;
+ struct ip_tunnel_info info;
+ bool metadata = false;
+ int err;
- if (data[IFLA_GENEVE_UDP_ZERO_CSUM6_RX] &&
- nla_get_u8(data[IFLA_GENEVE_UDP_ZERO_CSUM6_RX]))
- use_udp6_rx_checksums = false;
+ err = geneve_nl2info(dev, tb, data, &info, &metadata,
+ &use_udp6_rx_checksums, false);
+ if (err)
+ return err;
return geneve_configure(net, dev, &info, metadata, use_udp6_rx_checksums);
}
+static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
+ struct nlattr *data[])
+{
+ struct geneve_dev *geneve = netdev_priv(dev);
+ struct ip_tunnel_info info;
+ bool metadata = false;
+ bool use_udp6_rx_checksums = false;
+ int err;
+
+ err = geneve_nl2info(dev, tb, data, &info, &metadata,
+ &use_udp6_rx_checksums, true);
+ if (err)
+ return err;
+
+ if (!geneve_dst_addr_equal(&geneve->info, &info))
+ dst_cache_reset(&info.dst_cache);
+ geneve->info = info;
+ geneve->collect_md = metadata;
+ geneve->use_udp6_rx_checksums = use_udp6_rx_checksums;
+
+ return 0;
+}
+
static void geneve_dellink(struct net_device *dev, struct list_head *head)
{
struct geneve_dev *geneve = netdev_priv(dev);
@@ -1344,6 +1428,7 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
.setup = geneve_setup,
.validate = geneve_validate,
.newlink = geneve_newlink,
+ .changelink = geneve_changelink,
.dellink = geneve_dellink,
.get_size = geneve_get_size,
.fill_info = geneve_fill_info,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] geneve: add rtnl changelink support
From: David Miller @ 2017-05-08 20:18 UTC (permalink / raw)
To: girish.moodalbail; +Cc: netdev, pshelar, joe, jbenc
In-Reply-To: <1494274104-16494-1-git-send-email-girish.moodalbail@oracle.com>
From: Girish Moodalbail <girish.moodalbail@oracle.com>
Date: Mon, 8 May 2017 13:08:24 -0700
> This patch adds changelink rtnl operation support for geneve devices.
> Code changes involve:
> - refactor geneve_newlink into geneve_nl2info to be used by both
> geneve_newlink and geneve_changelink
> - geneve_nl2info takes a changelink boolean argument to isolate
> changelink checks and updates.
> - Allow changing only a few attributes:
> - return -EOPNOTSUPP for attributes that cannot be changed for
> now. Incremental patches can make the non-supported one
> available in the future if needed.
>
> Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
The net-next tree is closed, please resubmit this when the net-next
tree opens back up.
Thank you.
^ permalink raw reply
* Re: [PATCH net] ip6_tunnel: remove unreachable ICMP_REDIRECT code
From: Cong Wang @ 2017-05-08 20:26 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Linux Kernel Network Developers
In-Reply-To: <1494241863-32549-1-git-send-email-liuhangbin@gmail.com>
On Mon, May 8, 2017 at 4:11 AM, Hangbin Liu <liuhangbin@gmail.com> wrote:
> After call ip6_tnl_err(), the rel_type will be ether ICMPV6_DEST_UNREACH
> or ICMPV6_PKT_TOOBIG. We will never reach ICMP_REDIRECT. So remove it.
Are you sure we really don't need to handle NDISC_REDIRECT here?
I can't find anything in RFC 2473 explictly, but I am feeling we should handle
it rather than ignoring it according to:
To report a problem detected inside the tunnel to the source of an
original packet, the tunnel entry point node must relay the ICMP
message received from inside the tunnel to the source of that
original IPv6 packet.
I am not sure...
^ permalink raw reply
* 55346
From: ccc @ 2017-05-08 20:48 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 8.zip --]
[-- Type: application/zip, Size: 1866 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] TI Bluetooth serdev support
From: Rob Herring @ 2017-05-08 21:12 UTC (permalink / raw)
To: Adam Ford
Cc: Sebastian Reichel, Marcel Holtmann, open list:BLUETOOTH DRIVERS,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Johan Hedberg, Gustavo Padovan, Satish Patel, Wei Xu, Eyal Reizer,
netdev,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAHCN7x+Mq=b6RN-ezU0400W=H=D7DR+Es1FHtT4GyozpVd8ALA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, May 5, 2017 at 9:51 AM, Adam Ford <aford173-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Sun, Apr 30, 2017 at 11:04 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> Hi,
>>
>> On Sun, Apr 30, 2017 at 10:14:20AM -0500, Adam Ford wrote:
>>> On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> > This series adds serdev support to the HCI LL protocol used on TI BT
>>> > modules and enables support on HiKey board with with the WL1835 module.
>>> > With this the custom TI UIM daemon and btattach are no longer needed.
>>>
>>> Without UIM daemon, what instruction do you use to load the BT firmware?
>>>
>>> I was thinking 'hciattach' but I was having trouble. I was hoping you
>>> might have some insight.
>>>
>>> hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow Just
>>> returns a timeout.
>>>
>>> I modified my i.MX6 device tree per the binding documentation and
>>> setup the regulators and enable GPIO pins.
>>
>> If you configured everything correctly no userspace interaction is
>> required. The driver should request the firmware automatically once
>> you power up the bluetooth device.
>>
>> Apart from DT changes make sure, that the following options are
>> enabled and check dmesg for any hints.
>>
>> CONFIG_SERIAL_DEV_BUS
>> CONFIG_SERIAL_DEV_CTRL_TTYPORT
>> CONFIG_BT_HCIUART
>> CONFIG_BT_HCIUART_LL
>>
>
>
> I have enabled those flags, and I have updated my device tree.
> I am testing this on an OMAP3630 (DM3730) board with a WL1283. I am
> getting a lot of timeout errors. I tested this against the original
> implemention I had in pdata-quirks.c using the ti-st driver, uim & and
> the btwilink driver.
>
> I pulled in some of the newer patches to enable the wl1283-st, but I
> am obviously missing something.
>
> I 58.717651] Bluetooth: hci0: Reading TI version information failed
> (-110)
> [ 58.724853] Bluetooth: hci0: download firmware failed, retrying...
> [ 60.957641] Bluetooth: hci0 command 0x1001 tx timeout
> [ 68.957641] Bluetooth: hci0: Reading TI version information failed
> (-110)
> [ 68.964843] Bluetooth: hci0: download firmware failed, retrying...
> [ 69.132171] Bluetooth: Unknown HCI packet type 06
> [ 69.138244] Bluetooth: Unknown HCI packet type 0c
> [ 69.143249] Bluetooth: Unknown HCI packet type 40
> [ 69.148498] Bluetooth: Unknown HCI packet type 20
> [ 69.153533] Bluetooth: Data length is too large
> [ 69.158569] Bluetooth: Unknown HCI packet type a0
> [ 69.163574] Bluetooth: Unknown HCI packet type 00
> [ 69.168731] Bluetooth: Unknown HCI packet type 00
> [ 69.173736] Bluetooth: Unknown HCI packet type 34
> [ 69.178924] Bluetooth: Unknown HCI packet type 91
> [ 71.197631] Bluetooth: hci0 command 0x1001 tx timeout
> [ 79.197662] Bluetooth: hci0: Reading TI version information failed (-110)
There's a bug in serdev_device_write(), so if you have that function
you need either the fix I sent or the patch to make
serdev_device_writebuf atomic again. Both are on the linux-serial
list, but not in any tree yet.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] drivers: net: wimax: i2400m: i2400m-usb: Use time_after for time comparison
From: David Miller @ 2017-05-08 21:26 UTC (permalink / raw)
To: karim.eshapa; +Cc: inaky.perez-gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <1494262682-5401-1-git-send-email-karim.eshapa@gmail.com>
From: Karim Eshapa <karim.eshapa@gmail.com>
Date: Mon, 8 May 2017 18:58:02 +0200
> diff --git a/drivers/net/wimax/i2400m/i2400m-usb.h b/drivers/net/wimax/i2400m/i2400m-usb.h
> index 649ecad..6fc941c 100644
> --- a/drivers/net/wimax/i2400m/i2400m-usb.h
> +++ b/drivers/net/wimax/i2400m/i2400m-usb.h
> @@ -131,7 +131,7 @@ static inline int edc_inc(struct edc *edc, u16 max_err, u16 timeframe)
> unsigned long now;
>
> now = jiffies;
> - if (now - edc->timestart > timeframe) {
> + if (time_after(now - edc->timestart, (unsigned long)timeframe)) {
This is far from correct.
time_after() compares two "jiffies" timestamp values. The second
argument here is not a jiffies timestamp value.
^ permalink raw reply
* Re: [Patch net v3] ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf
From: David Miller @ 2017-05-08 21:32 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, andreyknvl, dsahern
In-Reply-To: <1494263533-22325-1-git-send-email-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 8 May 2017 10:12:13 -0700
> For each netns (except init_net), we initialize its null entry
> in 3 places:
>
> 1) The template itself, as we use kmemdup()
> 2) Code around dst_init_metrics() in ip6_route_net_init()
> 3) ip6_route_dev_notify(), which is supposed to initialize it after
> loopback registers
>
> Unfortunately the last one still happens in a wrong order because
> we expect to initialize net->ipv6.ip6_null_entry->rt6i_idev to
> net->loopback_dev's idev, thus we have to do that after we add
> idev to loopback. However, this notifier has priority == 0 same as
> ipv6_dev_notf, and ipv6_dev_notf is registered after
> ip6_route_dev_notifier so it is called actually after
> ip6_route_dev_notifier. This is similar to commit 2f460933f58e
> ("ipv6: initialize route null entry in addrconf_init()") which
> fixes init_net.
>
> Fix it by picking a smaller priority for ip6_route_dev_notifier.
> Also, we have to release the refcnt accordingly when unregistering
> loopback_dev because device exit functions are called before subsys
> exit functions.
>
> Acked-by: David Ahern <dsahern@gmail.com>
> Tested-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] net: ethernet: ti: cpsw: adjust cpsw fifos depth for fullduplex flow control
From: David Miller @ 2017-05-08 21:33 UTC (permalink / raw)
To: grygorii.strashko
Cc: netdev, nsekhar, ivan.khoronzhuk, linux-kernel, linux-omap,
spatton
In-Reply-To: <20170508192121.20451-1-grygorii.strashko@ti.com>
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Mon, 8 May 2017 14:21:21 -0500
> When users set flow control using ethtool the bits are set properly in the
> CPGMAC_SL MACCONTROL register, but the FIFO depth in the respective Port n
> Maximum FIFO Blocks (Pn_MAX_BLKS) registers remains set to the minimum size
> reset value. When receive flow control is enabled on a port, the port's
> associated FIFO block allocation must be adjusted. The port RX allocation
> must increase to accommodate the flow control runout. The TRM recommends
> numbers of 5 or 6.
>
> Hence, apply required Port FIFO configuration to
> Pn_MAX_BLKS.Pn_TX_MAX_BLKS=0xF and Pn_MAX_BLKS.Pn_RX_MAX_BLKS=0x5 during
> interface initialization.
>
> Cc: Schuyler Patton <spatton@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Applied.
^ permalink raw reply
* Re: [PATCH RFC net-next 3/6] net: Introduce IFF_LWT_NETDEV flag
From: Roopa Prabhu @ 2017-05-08 21:37 UTC (permalink / raw)
To: David Miller
Cc: Johannes Berg, David Ahern, netdev@vger.kernel.org,
Florian Fainelli, Nicolas Dichtel
In-Reply-To: <20170508.161130.1538627061428691500.davem@davemloft.net>
On Mon, May 8, 2017 at 1:11 PM, David Miller <davem@davemloft.net> wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Mon, 08 May 2017 10:55:12 +0200
>
>>
>>> +static inline bool netif_is_lwd(struct net_device *dev)
>>> +{
>>> + return !!(dev->priv_flags & IFF_LWT_NETDEV);
>>> +}
>>
>> Am I the only one who thinks that this "LWT_NETDEV" vs "LWD" is a bit
>> confusing?
>
> Agreed, my old eyes can't discern them at a distance :-)
agree.
mix of LWT_NETDEV and LWD can get confusing.
LWT already stands for Light Weight Tunnel...,
this can only be LWD or LWN ;)....if people don't confuse it with
some weekly news device :)
^ permalink raw reply
* [PATCH] net: mdio-mux: bcm-iproc: call mdiobus_free() in error path
From: Jon Mason @ 2017-05-08 21:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, linux-arm-kernel, linux-kernel, bcm-kernel-feedback-list
If an error is encountered in mdio_mux_init(), the error path will call
mdiobus_free(). Since mdiobus_register() has been called prior to
mdio_mux_init(), the bus->state will not be MDIOBUS_UNREGISTERED. This
causes a BUG_ON() in mdiobus_free(). To correct this issue, add an
error path for mdio_mux_init() which calls mdiobus_unregister() prior to
mdiobus_free().
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
Fixes: 98bc865a1ec8 ("net: mdio-mux: Add MDIO mux driver for iProc SoCs")
---
drivers/net/phy/mdio-mux-bcm-iproc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
index 0a0412524cec..0a5f62e0efcc 100644
--- a/drivers/net/phy/mdio-mux-bcm-iproc.c
+++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
@@ -203,11 +203,14 @@ static int mdio_mux_iproc_probe(struct platform_device *pdev)
&md->mux_handle, md, md->mii_bus);
if (rc) {
dev_info(md->dev, "mdiomux initialization failed\n");
- goto out;
+ goto out_register;
}
dev_info(md->dev, "iProc mdiomux registered\n");
return 0;
+
+out_register:
+ mdiobus_unregister(bus);
out:
mdiobus_free(bus);
return rc;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: mdio-mux: bcm-iproc: call mdiobus_free() in error path
From: Florian Fainelli @ 2017-05-08 21:53 UTC (permalink / raw)
To: Jon Mason, Florian Fainelli
Cc: netdev, linux-arm-kernel, linux-kernel, bcm-kernel-feedback-list
In-Reply-To: <1494280115-9252-1-git-send-email-jon.mason@broadcom.com>
On 05/08/2017 02:48 PM, Jon Mason wrote:
> If an error is encountered in mdio_mux_init(), the error path will call
> mdiobus_free(). Since mdiobus_register() has been called prior to
> mdio_mux_init(), the bus->state will not be MDIOBUS_UNREGISTERED. This
> causes a BUG_ON() in mdiobus_free(). To correct this issue, add an
> error path for mdio_mux_init() which calls mdiobus_unregister() prior to
> mdiobus_free().
>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> Fixes: 98bc865a1ec8 ("net: mdio-mux: Add MDIO mux driver for iProc SoCs")
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH] net: mdio-mux: bcm-iproc: call mdiobus_free() in error path
From: David Miller @ 2017-05-08 22:00 UTC (permalink / raw)
To: jon.mason
Cc: f.fainelli, netdev, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1494280115-9252-1-git-send-email-jon.mason@broadcom.com>
From: Jon Mason <jon.mason@broadcom.com>
Date: Mon, 8 May 2017 17:48:35 -0400
> If an error is encountered in mdio_mux_init(), the error path will call
> mdiobus_free(). Since mdiobus_register() has been called prior to
> mdio_mux_init(), the bus->state will not be MDIOBUS_UNREGISTERED. This
> causes a BUG_ON() in mdiobus_free(). To correct this issue, add an
> error path for mdio_mux_init() which calls mdiobus_unregister() prior to
> mdiobus_free().
>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> Fixes: 98bc865a1ec8 ("net: mdio-mux: Add MDIO mux driver for iProc SoCs")
Applied and queued up for -stable.
^ permalink raw reply
* [PATCH] DECnet: Use container_of() for embedded struct
From: Kees Cook @ 2017-05-08 22:31 UTC (permalink / raw)
To: David S. Miller; +Cc: linux-kernel, linux-decnet-user, netdev
Instead of a direct cross-type cast, use conatiner_of() to locate
the embedded structure, even in the face of future struct layout
randomization.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
net/decnet/dn_neigh.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/decnet/dn_neigh.c b/net/decnet/dn_neigh.c
index 482730cd8a56..eeb5fc561f80 100644
--- a/net/decnet/dn_neigh.c
+++ b/net/decnet/dn_neigh.c
@@ -110,7 +110,7 @@ struct neigh_table dn_neigh_table = {
static int dn_neigh_construct(struct neighbour *neigh)
{
struct net_device *dev = neigh->dev;
- struct dn_neigh *dn = (struct dn_neigh *)neigh;
+ struct dn_neigh *dn = container_of(neigh, struct dn_neigh, n);
struct dn_dev *dn_db;
struct neigh_parms *parms;
@@ -339,7 +339,7 @@ int dn_to_neigh_output(struct net *net, struct sock *sk, struct sk_buff *skb)
struct dst_entry *dst = skb_dst(skb);
struct dn_route *rt = (struct dn_route *) dst;
struct neighbour *neigh = rt->n;
- struct dn_neigh *dn = (struct dn_neigh *)neigh;
+ struct dn_neigh *dn = container_of(neigh, struct dn_neigh, n);
struct dn_dev *dn_db;
bool use_long;
@@ -391,7 +391,7 @@ int dn_neigh_router_hello(struct net *net, struct sock *sk, struct sk_buff *skb)
neigh = __neigh_lookup(&dn_neigh_table, &src, skb->dev, 1);
- dn = (struct dn_neigh *)neigh;
+ dn = container_of(neigh, struct dn_neigh, n);
if (neigh) {
write_lock(&neigh->lock);
@@ -451,7 +451,7 @@ int dn_neigh_endnode_hello(struct net *net, struct sock *sk, struct sk_buff *skb
neigh = __neigh_lookup(&dn_neigh_table, &src, skb->dev, 1);
- dn = (struct dn_neigh *)neigh;
+ dn = container_of(neigh, struct dn_neigh, n);
if (neigh) {
write_lock(&neigh->lock);
@@ -510,7 +510,7 @@ static void neigh_elist_cb(struct neighbour *neigh, void *_info)
if (neigh->dev != s->dev)
return;
- dn = (struct dn_neigh *) neigh;
+ dn = container_of(neigh, struct dn_neigh, n);
if (!(dn->flags & (DN_NDFLAG_R1|DN_NDFLAG_R2)))
return;
@@ -549,7 +549,7 @@ int dn_neigh_elist(struct net_device *dev, unsigned char *ptr, int n)
static inline void dn_neigh_format_entry(struct seq_file *seq,
struct neighbour *n)
{
- struct dn_neigh *dn = (struct dn_neigh *) n;
+ struct dn_neigh *dn = container_of(n, struct dn_neigh, n);
char buf[DN_ASCBUF_LEN];
read_lock(&n->lock);
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* [PATCH] drivers: net: wireless: rsi: rsi_91x_core: Use time_after time comparison
From: Karim Eshapa @ 2017-05-08 22:34 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel, Karim Eshapa
Use time_after kernel macro for time comparison.
Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
drivers/net/wireless/rsi/rsi_91x_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_core.c b/drivers/net/wireless/rsi/rsi_91x_core.c
index f3d3995..68f04a7 100644
--- a/drivers/net/wireless/rsi/rsi_91x_core.c
+++ b/drivers/net/wireless/rsi/rsi_91x_core.c
@@ -306,7 +306,7 @@ void rsi_core_qos_processor(struct rsi_common *common)
tstamp_2 = jiffies;
mutex_unlock(&common->tx_rxlock);
- if (tstamp_2 > tstamp_1 + (300 * HZ / 1000))
+ if (time_after(tstamp_2, tstamp_1 + (300 * HZ) / 1000))
schedule();
}
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] drivers: net: wimax: i2400m: i2400m-usb: Use time_after for time comparison
From: Girish Moodalbail @ 2017-05-08 23:27 UTC (permalink / raw)
To: David Miller, karim.eshapa
Cc: inaky.perez-gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <20170508.172618.676839687841750049.davem@davemloft.net>
On 5/8/17 2:26 PM, David Miller wrote:
> From: Karim Eshapa <karim.eshapa@gmail.com>
> Date: Mon, 8 May 2017 18:58:02 +0200
>
>> diff --git a/drivers/net/wimax/i2400m/i2400m-usb.h b/drivers/net/wimax/i2400m/i2400m-usb.h
>> index 649ecad..6fc941c 100644
>> --- a/drivers/net/wimax/i2400m/i2400m-usb.h
>> +++ b/drivers/net/wimax/i2400m/i2400m-usb.h
>> @@ -131,7 +131,7 @@ static inline int edc_inc(struct edc *edc, u16 max_err, u16 timeframe)
>> unsigned long now;
>>
>> now = jiffies;
>> - if (now - edc->timestart > timeframe) {
>> + if (time_after(now - edc->timestart, (unsigned long)timeframe)) {
>
> This is far from correct.
>
> time_after() compares two "jiffies" timestamp values. The second
> argument here is not a jiffies timestamp value.
>
Perhaps, what is needed here is:
+ if (time_after(jiffies, edc->timestart + timeframe)) {
where in 'timeframe' is set in HZ in all the callers I checked (for the most
part it is set to EDC_ERROR_TIMEFRAME which is 1HZ).
~Girish
^ permalink raw reply
* Re: [PATCH] net: dsa: loop: Check for memory allocation failure
From: Julia Lawall @ 2017-05-08 23:46 UTC (permalink / raw)
To: Joe Perches
Cc: David Laight, 'Christophe JAILLET', andrew@lunn.ch,
vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
In-Reply-To: <1494265443.31950.62.camel@perches.com>
On Mon, 8 May 2017, Joe Perches wrote:
> On Mon, 2017-05-08 at 20:32 +0800, Julia Lawall wrote:
> >
> > On Mon, 8 May 2017, David Laight wrote:
> >
> > > From: Christophe JAILLET
> > > > Sent: 06 May 2017 06:30
> > > > If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> > > > Return -ENOMEM instead, as done for some other memory allocation just a
> > > > few lines above.
> > >
> > > ...
> > > > --- a/drivers/net/dsa/dsa_loop.c
> > > > +++ b/drivers/net/dsa/dsa_loop.c
> > > > @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
> > > > return -ENOMEM;
> > > >
> > > > ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
> > > > + if (!ps)
> > > > + return -ENOMEM;
> > > > +
> > > > ps->netdev = dev_get_by_name(&init_net, pdata->netdev);
> > > > if (!ps->netdev)
> > > > return -EPROBE_DEFER;
> > >
> > > On the face if it this code leaks like a sieve.
> >
> > I don't think so. The allocations (dsa_switch_alloc and devm_kzalloc) use
> > devm functions.
>
> It's at least wasteful.
>
> Each time -EPROBE_DEFER occurs, another set of calls to
> dsa_switch_alloc and dev_kzalloc also occurs.
>
> Perhaps it'd be better to do:
>
> if (ps->netdev) {
> devm_kfree(&devmdev->dev, ps);
> devm_kfree(&mdiodev->dev, ds);
> return -EPROBE_DEFER;
> }
Is EPROBE_DEFER handled differently than other kinds of errors?
julia
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: Eric Dumazet @ 2017-05-09 0:01 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, netdev, andreyknvl, edumazet
In-Reply-To: <20170508.143557.105629611489969352.davem@davemloft.net>
On Mon, 2017-05-08 at 14:35 -0400, David Miller wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Thu, 4 May 2017 14:54:17 -0700
>
> > IPv4 dst could use fi->fib_metrics to store metrics but fib_info
> > itself is refcnt'ed, so without taking a refcnt fi and
> > fi->fib_metrics could be freed while dst metrics still points to
> > it. This triggers use-after-free as reported by Andrey twice.
> >
> > This patch reverts commit 2860583fe840 ("ipv4: Kill rt->fi") to
> > restore this reference counting. It is a quick fix for -net and
> > -stable, for -net-next, as Eric suggested, we can consider doing
> > reference counting for metrics itself instead of relying on fib_info.
> >
> > IPv6 is very different, it copies or steals the metrics from mx6_config
> > in fib6_commit_metrics() so probably doesn't need a refcnt.
> >
> > Decnet has already done the refcnt'ing, see dn_fib_semantic_match().
> >
> > Fixes: 2860583fe840 ("ipv4: Kill rt->fi")
> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> > Tested-by: Andrey Konovalov <andreyknvl@google.com>
> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> Applied and queued up for -stable, thanks.
Although I now have on latest net tree these messages when I reboot my
test machine.
[ 224.085873] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 234.106018] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 244.366187] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 254.626325] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 264.706472] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 274.736619] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 284.766766] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
^ permalink raw reply
* RE: [PATCH] drivers: net: wimax: i2400m: i2400m-usb: Use time_after for time comparison
From: Karim Eshapa @ 2017-05-09 0:06 UTC (permalink / raw)
To: inaky.perez-gonzalez; +Cc: linux-wimax, netdev, linux-kernel, Karim Eshapa
In-Reply-To: <1494262682-5401-1-git-send-email-karim.eshapa@gmail.com>
Use time_after() for time comparison with the new fix.
Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
drivers/net/wimax/i2400m/i2400m-usb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/i2400m-usb.h b/drivers/net/wimax/i2400m/i2400m-usb.h
index 649ecad..eff4f464 100644
--- a/drivers/net/wimax/i2400m/i2400m-usb.h
+++ b/drivers/net/wimax/i2400m/i2400m-usb.h
@@ -131,7 +131,7 @@ static inline int edc_inc(struct edc *edc, u16 max_err, u16 timeframe)
unsigned long now;
now = jiffies;
- if (now - edc->timestart > timeframe) {
+ if (time_after(now, edc->timestart + timeframe)) {
edc->errorcount = 1;
edc->timestart = now;
} else if (++edc->errorcount > max_err) {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: dsa: loop: Check for memory allocation failure
From: Florian Fainelli @ 2017-05-09 0:35 UTC (permalink / raw)
To: Julia Lawall, Joe Perches
Cc: David Laight, 'Christophe JAILLET', andrew@lunn.ch,
vivien.didelot@savoirfairelinux.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <alpine.DEB.2.20.1705090745190.3309@hadrien>
On 05/08/2017 04:46 PM, Julia Lawall wrote:
>
>
> On Mon, 8 May 2017, Joe Perches wrote:
>
>> On Mon, 2017-05-08 at 20:32 +0800, Julia Lawall wrote:
>>>
>>> On Mon, 8 May 2017, David Laight wrote:
>>>
>>>> From: Christophe JAILLET
>>>>> Sent: 06 May 2017 06:30
>>>>> If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
>>>>> Return -ENOMEM instead, as done for some other memory allocation just a
>>>>> few lines above.
>>>>
>>>> ...
>>>>> --- a/drivers/net/dsa/dsa_loop.c
>>>>> +++ b/drivers/net/dsa/dsa_loop.c
>>>>> @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
>>>>> return -ENOMEM;
>>>>>
>>>>> ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
>>>>> + if (!ps)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> ps->netdev = dev_get_by_name(&init_net, pdata->netdev);
>>>>> if (!ps->netdev)
>>>>> return -EPROBE_DEFER;
>>>>
>>>> On the face if it this code leaks like a sieve.
>>>
>>> I don't think so. The allocations (dsa_switch_alloc and devm_kzalloc) use
>>> devm functions.
>>
>> It's at least wasteful.
>>
>> Each time -EPROBE_DEFER occurs, another set of calls to
>> dsa_switch_alloc and dev_kzalloc also occurs.
>>
>> Perhaps it'd be better to do:
>>
>> if (ps->netdev) {
>> devm_kfree(&devmdev->dev, ps);
>> devm_kfree(&mdiodev->dev, ds);
>> return -EPROBE_DEFER;
>> }
>
> Is EPROBE_DEFER handled differently than other kinds of errors?
In the core device driver model, yes, EPROBE_DEFER is treated
differently than other errors because it puts the driver on a retry queue.
EPROBE_DEFER is already a slow and exceptional path, and this is a
mock-up driver, so I am not sure what value there is in trying to
balance devm_kzalloc() with corresponding devm_kfree()...
--
Florian
^ permalink raw reply
* Re: [PATCH] net: dsa: loop: Check for memory allocation failure
From: Julia Lawall @ 2017-05-09 0:39 UTC (permalink / raw)
To: Florian Fainelli
Cc: Joe Perches, David Laight, 'Christophe JAILLET',
andrew@lunn.ch, vivien.didelot@savoirfairelinux.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
In-Reply-To: <f9058de7-ef9e-f6eb-751d-72ffdce512bb@gmail.com>
On Mon, 8 May 2017, Florian Fainelli wrote:
> On 05/08/2017 04:46 PM, Julia Lawall wrote:
> >
> >
> > On Mon, 8 May 2017, Joe Perches wrote:
> >
> >> On Mon, 2017-05-08 at 20:32 +0800, Julia Lawall wrote:
> >>>
> >>> On Mon, 8 May 2017, David Laight wrote:
> >>>
> >>>> From: Christophe JAILLET
> >>>>> Sent: 06 May 2017 06:30
> >>>>> If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> >>>>> Return -ENOMEM instead, as done for some other memory allocation just a
> >>>>> few lines above.
> >>>>
> >>>> ...
> >>>>> --- a/drivers/net/dsa/dsa_loop.c
> >>>>> +++ b/drivers/net/dsa/dsa_loop.c
> >>>>> @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
> >>>>> return -ENOMEM;
> >>>>>
> >>>>> ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
> >>>>> + if (!ps)
> >>>>> + return -ENOMEM;
> >>>>> +
> >>>>> ps->netdev = dev_get_by_name(&init_net, pdata->netdev);
> >>>>> if (!ps->netdev)
> >>>>> return -EPROBE_DEFER;
> >>>>
> >>>> On the face if it this code leaks like a sieve.
> >>>
> >>> I don't think so. The allocations (dsa_switch_alloc and devm_kzalloc) use
> >>> devm functions.
> >>
> >> It's at least wasteful.
> >>
> >> Each time -EPROBE_DEFER occurs, another set of calls to
> >> dsa_switch_alloc and dev_kzalloc also occurs.
> >>
> >> Perhaps it'd be better to do:
> >>
> >> if (ps->netdev) {
> >> devm_kfree(&devmdev->dev, ps);
> >> devm_kfree(&mdiodev->dev, ds);
> >> return -EPROBE_DEFER;
> >> }
> >
> > Is EPROBE_DEFER handled differently than other kinds of errors?
>
> In the core device driver model, yes, EPROBE_DEFER is treated
> differently than other errors because it puts the driver on a retry queue.
>
> EPROBE_DEFER is already a slow and exceptional path, and this is a
> mock-up driver, so I am not sure what value there is in trying to
> balance devm_kzalloc() with corresponding devm_kfree()...
OK, thanks for the explanation.
julia
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Casey Leedom @ 2017-05-09 0:48 UTC (permalink / raw)
To: Alexander Duyck, Ding Tianhong
Cc: Raj, Ashok, Bjorn Helgaas, Michael Werner, Ganesh GR, Arjun V.,
Asit K Mallick, Patrick J Cramer, Suravee Suthikulpanit, Bob Shaw,
h, Mark Rutland, Amir Ancel, Gabriele Paoloni, Catalin Marinas,
Will Deacon, LinuxArm, David Laight, Jeff Kirsher, Netdev,
"Robin Murphy" <robin.mu
In-Reply-To: <CAKgT0Ufg6uebqj79e1FOHoPpF-GxPYMkp353r3Ok_=nM8wF6qA@mail.gmail.com>
| From: Alexander Duyck <alexander.duyck@gmail.com>
| Date: Saturday, May 6, 2017 11:07 AM
|
| | From: Ding Tianhong <dingtianhong@huawei.com>
| | Date: Fri, May 5, 2017 at 8:08 PM
| |
| | According the suggestion, I could only think of this code:
| | ..
|
| This is a bit simplistic but it is a start.
Yes, something tells me that this is going to be more complicated than any
of us like ...
| The other bit I was getting at is that we need to update the core PCIe
| code so that when we configure devices and the root complex reports no
| support for relaxed ordering it should be clearing the relaxed
| ordering bits in the PCIe configuration registers on the upstream
| facing devices.
Of course, this can be written to by the Driver at any time ... and is in
the case of the cxgb4 Driver ...
After a lot of rummaging around, it does look like KVM prohibits writes to
the PCIe Capability Device Control register in drivers/xen/xen-pciback/
conf_space_capability.c and conf_space.c simply because writes aren't
allowed unless "permissive" is set. So it ~looks~ like a driver running in
a Virtual Machine can't turn Enable Relaxed Ordering back on ...
| The last bit we need in all this is a way to allow for setups where
| peer-to-peer wants to perform relaxed ordering but for writes to the
| host we have to not use relaxed ordering. For that we need to enable a
| special case and that isn't handled right now in any of the solutions
| we have coded up so far.
Yes, we do need this.
| From: Alexander Duyck <alexander.duyck@gmail.com>
| Date: Saturday, May 8, 2017 08:22 AM
|
| The problem is we need to have something that can be communicated
| through a VM. Your change doesn't work in that regard. That was why I
| suggested just updating the code so that we when we initialized PCIe
| devices what we do is either set or clear the relaxed ordering bit in
| the PCIe device control register. That way when we direct assign an
| interface it could know just based on the bits int the PCIe
| configuration if it could use relaxed ordering or not.
|
| At that point the driver code itself becomes very simple since you
| could just enable the relaxed ordering by default in the igb/ixgbe
| driver and if the bit is set or cleared in the PCIe configuration then
| we are either sending with relaxed ordering requests or not and don't
| have to try and locate the root complex.
|
| So from the sound of it Casey has a special use case where he doesn't
| want to send relaxed ordering frames to the root complex, but instead
| would like to send them to another PCIe device. To do that he needs to
| have a way to enable the relaxed ordering bit in the PCIe
| configuration but then not send any to the root complex. Odds are that
| is something he might be able to just implement in the driver, but is
| something that may become a more general case in the future. I don't
| see our change here impacting it as long as we keep the solution
| generic and mostly confined to when we instantiate the devices as the
| driver could likely make the decision to change the behavior later.
It's not just me. Intel has said that while RO directed at the Root
Complex Host Coherent Memory has a performance bug (not Data Corruption),
it's a performance win for Peer-to-Peer writes to MMIO Space. (I'll be very
interested in hearing what the bug is if we get that much detail. The very
same TLPs directed to the Root Complex Port without Relaxed Ordering set get
good performance. So this is essentially a bug in the hardware that was
~trying~ to implement a performance win.)
Meanwhile, I currently only know of a single PCIe End Point which causes
catastrophic results: the AMD A1100 ARM SoC ("SEATTLE"). And it's not even
clear that product is even alive anymore since I haven't been able to get
any responses from them for several months.
What I'm saying is: let's try to architect a solution which doesn't throw
the baby out with the bath water ...
I think that if a Device's Root Complex Port has problems with Relaxed
Ordering, it ~probably~ makes sense to turn off the PCIe Capability Device
Control[Enable Relaxed Ordering] when we assign a device to a Virtual
Machine since the Device Driver can no longer query the Relaxed Ordering
Support of the Root Complex Port. The only down side of this would be if we
assigned two Peers to a VM in an application which wanted to do Peer-to-Peer
transfers. But that seems like a hard application to support in any case
since the PCI Bus:Slot.Function IDs for assigned Devices within a VM don't
match the actual values.
For Devices running in the base OS/Hypervisor, their Drivers can query the
Relaxed Ordering Support for the Root Complex Port or a Peer Device. So a
simple flag within the (struct pci_dev *)->dev_flags would serve for that
along with a per-Architecture/Platform mechanism for setting it ...
Casey
^ permalink raw reply
* Re: [PATCH RFC net-next 3/6] net: Introduce IFF_LWT_NETDEV flag
From: David Ahern @ 2017-05-09 0:57 UTC (permalink / raw)
To: David Miller, johannes; +Cc: netdev, roopa, f.fainelli, nicolas.dichtel
In-Reply-To: <20170508.161130.1538627061428691500.davem@davemloft.net>
On 5/8/17 1:11 PM, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Mon, 08 May 2017 10:55:12 +0200
>
>>
>>> +static inline bool netif_is_lwd(struct net_device *dev)
>>> +{
>>> + return !!(dev->priv_flags & IFF_LWT_NETDEV);
>>> +}
>>
>> Am I the only one who thinks that this "LWT_NETDEV" vs "LWD" is a bit
>> confusing?
>
> Agreed, my old eyes can't discern them at a distance :-)
>
perhaps it is the tiny font your old eyes are having trouble with :-)
I am fine with Johannes' suggestion -- just spell it out:
netif_is_lwt_netdev
where lwt = LightWeighT
^ 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