[SRU][Trusty][PATCH 0/5] Fixes for LP:1690094

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH 0/5] Fixes for LP:1690094

Joseph Salisbury-3
BugLink: http://bugs.launchpad.net/bugs/1690094

== SRU Justification ==

Netlink notification is missing when an interface is modified on kernels
older that 3.18.  These commits are only needed in Trusty.  An example of
this bug is:

ip monitor link&
ip link set eth1 txqueuelen 18

=> no notification

This bugs is fixed by four commits, which also require on commit as a prereq,
which is commit 7d49b1f0f8e5.

These commits were included in maline as follows:
7d49b1f0f8e5f24294a880ed576964059af5ef - v3.14-rc1~94^2~25
5d1180fcacc5ceb7da5494acfe9c5e4ebad4f2 - v3.18-rc1~52^2~173^2~3
1889b0e7efe8373793069bd3deb7702a51e6f2 - v3.18-rc1~52^2~173^2~2
90c325e3bfe14ef360de6650fa2a2e92685e5c - v3.18-rc1~52^2~173^2~1
ba9989069f4e426b1e0ed7018eacc9e1ba6070 - v3.18-rc1~52^2~173^2

== Fixes ==
commit ba7d49b1f0f8e5f24294a880ed576964059af5ef
Author: Jiri Pirko <[hidden email]>
Date:   Wed Jan 22 09:05:55 2014 +0100

    rtnetlink: provide api for getting and setting slave info

commit 5d1180fcacc5ceb7da5494acfe9c5e4ebad4f281
Author: Nicolas Dichtel <[hidden email]>
Date:   Mon Sep 1 16:07:26 2014 +0200

    rtnl/do_setlink(): set modified when IFLA_TXQLEN is updated

commit 1889b0e7efe8373793069bd3deb7702a51e6f2a5
Author: Nicolas Dichtel <[hidden email]>
Date:   Mon Sep 1 16:07:27 2014 +0200

    rtnl/do_setlink(): set modified when IFLA_LINKMODE is updated

commit 90c325e3bfe14ef360de6650fa2a2e92685e5cee
Author: Nicolas Dichtel <[hidden email]>
Date:   Mon Sep 1 16:07:28 2014 +0200

    rtnl/do_setlink(): last arg is now a set of flags

commit ba9989069f4e426b1e0ed7018eacc9e1ba607095
Author: Nicolas Dichtel <[hidden email]>
Date:   Mon Sep 1 16:07:29 2014 +0200

    rtnl/do_setlink(): notify when a netdev is modified

== Regression Potential ==
There is a chance for regression due to the number of changes.  However, two of
the patches are to print warning messages.


== Test Case ==
A test kernel was built with these patches and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Jiri Pirko (1):
  rtnetlink: provide api for getting and setting slave info

Nicolas Dichtel (4):
  rtnl/do_setlink(): set modified when IFLA_TXQLEN is updated
  rtnl/do_setlink(): set modified when IFLA_LINKMODE is updated
  rtnl/do_setlink(): last arg is now a set of flags
  rtnl/do_setlink(): notify when a netdev is modified

 include/net/rtnetlink.h      |  14 +++
 include/uapi/linux/if_link.h |   2 +
 net/core/rtnetlink.c         | 219 +++++++++++++++++++++++++++++++++++--------
 3 files changed, 195 insertions(+), 40 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH 1/5] rtnetlink: provide api for getting and setting slave info

Joseph Salisbury-3
From: Jiri Pirko <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1690094

Recent patch
bonding: add netlink attributes to slave link dev (1d3ee88ae0d6)

Introduced yet another device specific way to access slave information
over rtnetlink. There is one already there for bridge.

This patch introduces generic way to do this, for getting and setting
info as well by extending link_ops. Later on, this new interface will
be used for bridge ports as well.

Signed-off-by: Jiri Pirko <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(backported from commit ba7d49b1f0f8e5f24294a880ed576964059af5ef)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 include/net/rtnetlink.h      |  14 ++++
 include/uapi/linux/if_link.h |   2 +
 net/core/rtnetlink.c         | 155 ++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 154 insertions(+), 17 deletions(-)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index bb13a18..59830f9 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -79,6 +79,20 @@ struct rtnl_link_ops {
        const struct net_device *dev);
  unsigned int (*get_num_tx_queues)(void);
  unsigned int (*get_num_rx_queues)(void);
+
+ int slave_maxtype;
+ const struct nla_policy *slave_policy;
+ int (*slave_validate)(struct nlattr *tb[],
+  struct nlattr *data[]);
+ int (*slave_changelink)(struct net_device *dev,
+    struct net_device *slave_dev,
+    struct nlattr *tb[],
+    struct nlattr *data[]);
+ size_t (*get_slave_size)(const struct net_device *dev,
+  const struct net_device *slave_dev);
+ int (*fill_slave_info)(struct sk_buff *skb,
+   const struct net_device *dev,
+   const struct net_device *slave_dev);
 };
 
 int __rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 6db4601..8a6063a 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -240,6 +240,8 @@ enum {
  IFLA_INFO_KIND,
  IFLA_INFO_DATA,
  IFLA_INFO_XSTATS,
+ IFLA_INFO_SLAVE_KIND,
+ IFLA_INFO_SLAVE_DATA,
  __IFLA_INFO_MAX,
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ba9a9af..d8638b9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -396,6 +396,22 @@ void rtnl_link_unregister(struct rtnl_link_ops *ops)
 }
 EXPORT_SYMBOL_GPL(rtnl_link_unregister);
 
+static size_t rtnl_link_get_slave_info_data_size(const struct net_device *dev)
+{
+ struct net_device *master_dev;
+ const struct rtnl_link_ops *ops;
+
+ master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
+ if (!master_dev)
+ return 0;
+ ops = master_dev->rtnl_link_ops;
+ if (!ops->get_slave_size)
+ return 0;
+ /* IFLA_INFO_SLAVE_DATA + nested data */
+ return nla_total_size(sizeof(struct nlattr)) +
+       ops->get_slave_size(master_dev, dev);
+}
+
 static size_t rtnl_link_get_size(const struct net_device *dev)
 {
  const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
@@ -416,6 +432,8 @@ static size_t rtnl_link_get_size(const struct net_device *dev)
  /* IFLA_INFO_XSTATS */
  size += nla_total_size(ops->get_xstats_size(dev));
 
+ size += rtnl_link_get_slave_info_data_size(dev);
+
  return size;
 }
 
@@ -508,40 +526,101 @@ static size_t rtnl_link_get_af_size(const struct net_device *dev)
  return size;
 }
 
-static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
+static bool rtnl_have_link_slave_info(const struct net_device *dev)
 {
- const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
- struct nlattr *linkinfo, *data;
- int err = -EMSGSIZE;
+ struct net_device *master_dev;
 
- linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
- if (linkinfo == NULL)
- goto out;
+ master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
+ if (master_dev && master_dev->rtnl_link_ops &&
+    master_dev->rtnl_link_ops->fill_slave_info)
+ return true;
+ return false;
+}
+
+static int rtnl_link_slave_info_fill(struct sk_buff *skb,
+     const struct net_device *dev)
+{
+ struct net_device *master_dev;
+ const struct rtnl_link_ops *ops;
+ struct nlattr *slave_data;
+ int err;
+
+ master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
+ if (!master_dev)
+ return 0;
+ ops = master_dev->rtnl_link_ops;
+ if (!ops)
+ return 0;
+ if (nla_put_string(skb, IFLA_INFO_SLAVE_KIND, ops->kind) < 0)
+ return -EMSGSIZE;
+ if (ops->fill_slave_info) {
+ slave_data = nla_nest_start(skb, IFLA_INFO_SLAVE_DATA);
+ if (!slave_data)
+ return -EMSGSIZE;
+ err = ops->fill_slave_info(skb, master_dev, dev);
+ if (err < 0)
+ goto err_cancel_slave_data;
+ nla_nest_end(skb, slave_data);
+ }
+ return 0;
+
+err_cancel_slave_data:
+ nla_nest_cancel(skb, slave_data);
+ return err;
+}
 
+static int rtnl_link_info_fill(struct sk_buff *skb,
+       const struct net_device *dev)
+{
+ const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
+ struct nlattr *data;
+ int err;
+
+ if (!ops)
+ return 0;
  if (nla_put_string(skb, IFLA_INFO_KIND, ops->kind) < 0)
- goto err_cancel_link;
+ return -EMSGSIZE;
  if (ops->fill_xstats) {
  err = ops->fill_xstats(skb, dev);
  if (err < 0)
- goto err_cancel_link;
+ return err;
  }
  if (ops->fill_info) {
  data = nla_nest_start(skb, IFLA_INFO_DATA);
- if (data == NULL) {
- err = -EMSGSIZE;
- goto err_cancel_link;
- }
+ if (data == NULL)
+ return -EMSGSIZE;
  err = ops->fill_info(skb, dev);
  if (err < 0)
  goto err_cancel_data;
  nla_nest_end(skb, data);
  }
-
- nla_nest_end(skb, linkinfo);
  return 0;
 
 err_cancel_data:
  nla_nest_cancel(skb, data);
+ return err;
+}
+
+static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
+{
+ struct nlattr *linkinfo;
+ int err = -EMSGSIZE;
+
+ linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
+ if (linkinfo == NULL)
+ goto out;
+
+ err = rtnl_link_info_fill(skb, dev);
+ if (err < 0)
+ goto err_cancel_link;
+
+ err = rtnl_link_slave_info_fill(skb, dev);
+ if (err < 0)
+ goto err_cancel_link;
+
+ nla_nest_end(skb, linkinfo);
+ return 0;
+
 err_cancel_link:
  nla_nest_cancel(skb, linkinfo);
 out:
@@ -1057,7 +1136,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
  if (rtnl_port_fill(skb, dev, ext_filter_mask))
  goto nla_put_failure;
 
- if (dev->rtnl_link_ops) {
+ if (dev->rtnl_link_ops || rtnl_have_link_slave_info(dev)) {
  if (rtnl_link_fill(skb, dev) < 0)
  goto nla_put_failure;
  }
@@ -1197,6 +1276,8 @@ EXPORT_SYMBOL(ifla_policy);
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
  [IFLA_INFO_KIND] = { .type = NLA_STRING },
  [IFLA_INFO_DATA] = { .type = NLA_NESTED },
+ [IFLA_INFO_SLAVE_KIND] = { .type = NLA_STRING },
+ [IFLA_INFO_SLAVE_DATA] = { .type = NLA_NESTED },
 };
 
 static const struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
@@ -1784,7 +1865,9 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
  struct net *net = sock_net(skb->sk);
  const struct rtnl_link_ops *ops;
+ const struct rtnl_link_ops *m_ops = NULL;
  struct net_device *dev;
+ struct net_device *master_dev = NULL;
  struct ifinfomsg *ifm;
  char kind[MODULE_NAME_LEN];
  char ifname[IFNAMSIZ];
@@ -1814,6 +1897,12 @@ replay:
  dev = NULL;
  }
 
+ if (dev) {
+ master_dev = netdev_master_upper_dev_get(dev);
+ if (master_dev)
+ m_ops = master_dev->rtnl_link_ops;
+ }
+
  err = validate_linkmsg(dev, tb);
  if (err < 0)
  return err;
@@ -1835,7 +1924,10 @@ replay:
  }
 
  if (1) {
- struct nlattr *attr[ops ? ops->maxtype + 1 : 0], **data = NULL;
+ struct nlattr *attr[ops ? ops->maxtype + 1 : 0];
+ struct nlattr *slave_attr[m_ops ? m_ops->slave_maxtype + 1 : 0];
+ struct nlattr **data = NULL;
+ struct nlattr **slave_data = NULL;
  struct net *dest_net;
 
  if (ops) {
@@ -1854,6 +1946,24 @@ replay:
  }
  }
 
+ if (m_ops) {
+ if (m_ops->slave_maxtype &&
+    linkinfo[IFLA_INFO_SLAVE_DATA]) {
+ err = nla_parse_nested(slave_attr,
+       m_ops->slave_maxtype,
+       linkinfo[IFLA_INFO_SLAVE_DATA],
+       m_ops->slave_policy);
+ if (err < 0)
+ return err;
+ slave_data = slave_attr;
+ }
+ if (m_ops->slave_validate) {
+ err = m_ops->slave_validate(tb, slave_data);
+ if (err < 0)
+ return err;
+ }
+ }
+
  if (dev) {
  int modified = 0;
 
@@ -1873,6 +1983,17 @@ replay:
  modified = 1;
  }
 
+ if (linkinfo[IFLA_INFO_SLAVE_DATA]) {
+ if (!m_ops || !m_ops->slave_changelink)
+ return -EOPNOTSUPP;
+
+ err = m_ops->slave_changelink(master_dev, dev,
+      tb, slave_data);
+ if (err < 0)
+ return err;
+ modified = 1;
+ }
+
  return do_setlink(skb, dev, ifm, tb, ifname, modified);
  }
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH 2/5] rtnl/do_setlink(): set modified when IFLA_TXQLEN is updated

Joseph Salisbury-3
In reply to this post by Joseph Salisbury-3
From: Nicolas Dichtel <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1690094

The only effect of this patch is to print a warning if IFLA_TXQLEN is updated
and a following change fails.

Signed-off-by: Nicolas Dichtel <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 5d1180fcacc5ceb7da5494acfe9c5e4ebad4f281)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 net/core/rtnetlink.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d8638b9..6b48fca 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1582,8 +1582,14 @@ static int do_setlink(const struct sk_buff *skb,
  modified = 1;
  }
 
- if (tb[IFLA_TXQLEN])
- dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
+ if (tb[IFLA_TXQLEN]) {
+ unsigned long value = nla_get_u32(tb[IFLA_TXQLEN]);
+
+ if (dev->tx_queue_len ^ value)
+ modified = 1;
+
+ dev->tx_queue_len = value;
+ }
 
  if (tb[IFLA_OPERSTATE])
  set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH 3/5] rtnl/do_setlink(): set modified when IFLA_LINKMODE is updated

Joseph Salisbury-3
In reply to this post by Joseph Salisbury-3
From: Nicolas Dichtel <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1690094

The only effect of this patch is to print a warning if IFLA_LINKMODE is updated
and a following change fails.

Signed-off-by: Nicolas Dichtel <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 1889b0e7efe8373793069bd3deb7702a51e6f2a5)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 net/core/rtnetlink.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 6b48fca..6448d35 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1595,8 +1595,12 @@ static int do_setlink(const struct sk_buff *skb,
  set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
 
  if (tb[IFLA_LINKMODE]) {
+ unsigned char value = nla_get_u8(tb[IFLA_LINKMODE]);
+
  write_lock_bh(&dev_base_lock);
- dev->link_mode = nla_get_u8(tb[IFLA_LINKMODE]);
+ if (dev->link_mode ^ value)
+ modified = 1;
+ dev->link_mode = value;
  write_unlock_bh(&dev_base_lock);
  }
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH 4/5] rtnl/do_setlink(): last arg is now a set of flags

Joseph Salisbury-3
In reply to this post by Joseph Salisbury-3
From: Nicolas Dichtel <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1690094

There is no functional changes with this commit, it only prepares the next one.

Signed-off-by: Nicolas Dichtel <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 90c325e3bfe14ef360de6650fa2a2e92685e5cee)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 net/core/rtnetlink.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 6448d35..7b45e74 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1451,9 +1451,10 @@ static int do_set_master(struct net_device *dev, int ifindex)
  return 0;
 }
 
+#define DO_SETLINK_MODIFIED 0x01
 static int do_setlink(const struct sk_buff *skb,
       struct net_device *dev, struct ifinfomsg *ifm,
-      struct nlattr **tb, char *ifname, int modified)
+      struct nlattr **tb, char *ifname, int status)
 {
  const struct net_device_ops *ops = dev->netdev_ops;
  int err;
@@ -1473,7 +1474,7 @@ static int do_setlink(const struct sk_buff *skb,
  put_net(net);
  if (err)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_MAP]) {
@@ -1502,7 +1503,7 @@ static int do_setlink(const struct sk_buff *skb,
  if (err < 0)
  goto errout;
 
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_ADDRESS]) {
@@ -1522,19 +1523,19 @@ static int do_setlink(const struct sk_buff *skb,
  kfree(sa);
  if (err)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_MTU]) {
  err = dev_set_mtu(dev, nla_get_u32(tb[IFLA_MTU]));
  if (err < 0)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_GROUP]) {
  dev_set_group(dev, nla_get_u32(tb[IFLA_GROUP]));
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  /*
@@ -1546,7 +1547,7 @@ static int do_setlink(const struct sk_buff *skb,
  err = dev_change_name(dev, ifname);
  if (err < 0)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_IFALIAS]) {
@@ -1554,7 +1555,7 @@ static int do_setlink(const struct sk_buff *skb,
     nla_len(tb[IFLA_IFALIAS]));
  if (err < 0)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_BROADCAST]) {
@@ -1572,21 +1573,21 @@ static int do_setlink(const struct sk_buff *skb,
  err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]));
  if (err)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_CARRIER]) {
  err = dev_change_carrier(dev, nla_get_u8(tb[IFLA_CARRIER]));
  if (err)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_TXQLEN]) {
  unsigned long value = nla_get_u32(tb[IFLA_TXQLEN]);
 
  if (dev->tx_queue_len ^ value)
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
 
  dev->tx_queue_len = value;
  }
@@ -1599,7 +1600,7 @@ static int do_setlink(const struct sk_buff *skb,
 
  write_lock_bh(&dev_base_lock);
  if (dev->link_mode ^ value)
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  dev->link_mode = value;
  write_unlock_bh(&dev_base_lock);
  }
@@ -1622,7 +1623,7 @@ static int do_setlink(const struct sk_buff *skb,
  err = do_setvfinfo(dev, vfinfo);
  if (err < 0)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
  }
  err = 0;
@@ -1652,7 +1653,7 @@ static int do_setlink(const struct sk_buff *skb,
  err = ops->ndo_set_vf_port(dev, vf, port);
  if (err < 0)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
  }
  err = 0;
@@ -1670,7 +1671,7 @@ static int do_setlink(const struct sk_buff *skb,
  err = ops->ndo_set_vf_port(dev, PORT_SELF_VF, port);
  if (err < 0)
  goto errout;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (tb[IFLA_AF_SPEC]) {
@@ -1687,13 +1688,13 @@ static int do_setlink(const struct sk_buff *skb,
  if (err < 0)
  goto errout;
 
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
  }
  err = 0;
 
 errout:
- if (err < 0 && modified)
+ if (err < 0 && status & DO_SETLINK_MODIFIED)
  net_warn_ratelimited("A link change request failed with some changes committed already. Interface %s may have been left with an inconsistent configuration, please check.\n",
      dev->name);
 
@@ -1975,7 +1976,7 @@ replay:
  }
 
  if (dev) {
- int modified = 0;
+ int status = 0;
 
  if (nlh->nlmsg_flags & NLM_F_EXCL)
  return -EEXIST;
@@ -1990,7 +1991,7 @@ replay:
  err = ops->changelink(dev, tb, data);
  if (err < 0)
  return err;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
  if (linkinfo[IFLA_INFO_SLAVE_DATA]) {
@@ -2001,10 +2002,10 @@ replay:
       tb, slave_data);
  if (err < 0)
  return err;
- modified = 1;
+ status |= DO_SETLINK_MODIFIED;
  }
 
- return do_setlink(skb, dev, ifm, tb, ifname, modified);
+ return do_setlink(skb, dev, ifm, tb, ifname, status);
  }
 
  if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH 5/5] rtnl/do_setlink(): notify when a netdev is modified

Joseph Salisbury-3
In reply to this post by Joseph Salisbury-3
From: Nicolas Dichtel <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1690094

Depending on which parameters were updated, the changes were not propagated via
the notifier chain and netlink.

The new flag has been set only when the change did not cause a call to the
notifier chain and/or to the netlink notification functions.

Signed-off-by: Nicolas Dichtel <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit ba9989069f4e426b1e0ed7018eacc9e1ba607095)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 net/core/rtnetlink.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7b45e74..f32efab 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1452,6 +1452,8 @@ static int do_set_master(struct net_device *dev, int ifindex)
 }
 
 #define DO_SETLINK_MODIFIED 0x01
+/* notify flag means notify + modified. */
+#define DO_SETLINK_NOTIFY 0x03
 static int do_setlink(const struct sk_buff *skb,
       struct net_device *dev, struct ifinfomsg *ifm,
       struct nlattr **tb, char *ifname, int status)
@@ -1503,7 +1505,7 @@ static int do_setlink(const struct sk_buff *skb,
  if (err < 0)
  goto errout;
 
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
 
  if (tb[IFLA_ADDRESS]) {
@@ -1535,7 +1537,7 @@ static int do_setlink(const struct sk_buff *skb,
 
  if (tb[IFLA_GROUP]) {
  dev_set_group(dev, nla_get_u32(tb[IFLA_GROUP]));
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
 
  /*
@@ -1555,7 +1557,7 @@ static int do_setlink(const struct sk_buff *skb,
     nla_len(tb[IFLA_IFALIAS]));
  if (err < 0)
  goto errout;
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
 
  if (tb[IFLA_BROADCAST]) {
@@ -1587,7 +1589,7 @@ static int do_setlink(const struct sk_buff *skb,
  unsigned long value = nla_get_u32(tb[IFLA_TXQLEN]);
 
  if (dev->tx_queue_len ^ value)
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
 
  dev->tx_queue_len = value;
  }
@@ -1600,7 +1602,7 @@ static int do_setlink(const struct sk_buff *skb,
 
  write_lock_bh(&dev_base_lock);
  if (dev->link_mode ^ value)
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  dev->link_mode = value;
  write_unlock_bh(&dev_base_lock);
  }
@@ -1623,7 +1625,7 @@ static int do_setlink(const struct sk_buff *skb,
  err = do_setvfinfo(dev, vfinfo);
  if (err < 0)
  goto errout;
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
  }
  err = 0;
@@ -1653,7 +1655,7 @@ static int do_setlink(const struct sk_buff *skb,
  err = ops->ndo_set_vf_port(dev, vf, port);
  if (err < 0)
  goto errout;
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
  }
  err = 0;
@@ -1671,7 +1673,7 @@ static int do_setlink(const struct sk_buff *skb,
  err = ops->ndo_set_vf_port(dev, PORT_SELF_VF, port);
  if (err < 0)
  goto errout;
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
 
  if (tb[IFLA_AF_SPEC]) {
@@ -1688,15 +1690,20 @@ static int do_setlink(const struct sk_buff *skb,
  if (err < 0)
  goto errout;
 
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
  }
  err = 0;
 
 errout:
- if (err < 0 && status & DO_SETLINK_MODIFIED)
- net_warn_ratelimited("A link change request failed with some changes committed already. Interface %s may have been left with an inconsistent configuration, please check.\n",
-     dev->name);
+ if (status & DO_SETLINK_MODIFIED) {
+ if (status & DO_SETLINK_NOTIFY)
+ netdev_state_change(dev);
+
+ if (err < 0)
+ net_warn_ratelimited("A link change request failed with some changes committed already. Interface %s may have been left with an inconsistent configuration, please check.\n",
+     dev->name);
+ }
 
  return err;
 }
@@ -1991,7 +1998,7 @@ replay:
  err = ops->changelink(dev, tb, data);
  if (err < 0)
  return err;
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
 
  if (linkinfo[IFLA_INFO_SLAVE_DATA]) {
@@ -2002,7 +2009,7 @@ replay:
       tb, slave_data);
  if (err < 0)
  return err;
- status |= DO_SETLINK_MODIFIED;
+ status |= DO_SETLINK_NOTIFY;
  }
 
  return do_setlink(skb, dev, ifm, tb, ifname, status);
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [SRU][Trusty][PATCH 0/5] Fixes for LP:1690094

Stefan Bader-2
In reply to this post by Joseph Salisbury-3
On 02.06.2017 17:19, Joseph Salisbury wrote:

> BugLink: http://bugs.launchpad.net/bugs/1690094
>
> == SRU Justification ==
>
> Netlink notification is missing when an interface is modified on kernels
> older that 3.18.  These commits are only needed in Trusty.  An example of
> this bug is:
>
> ip monitor link&
> ip link set eth1 txqueuelen 18
>
> => no notification
>
> This bugs is fixed by four commits, which also require on commit as a prereq,
> which is commit 7d49b1f0f8e5.
>
> These commits were included in maline as follows:
> 7d49b1f0f8e5f24294a880ed576964059af5ef - v3.14-rc1~94^2~25
> 5d1180fcacc5ceb7da5494acfe9c5e4ebad4f2 - v3.18-rc1~52^2~173^2~3
> 1889b0e7efe8373793069bd3deb7702a51e6f2 - v3.18-rc1~52^2~173^2~2
> 90c325e3bfe14ef360de6650fa2a2e92685e5c - v3.18-rc1~52^2~173^2~1
> ba9989069f4e426b1e0ed7018eacc9e1ba6070 - v3.18-rc1~52^2~173^2
Moving forward I would like to have the SRU justification in the bug report and
be geared to assess the risk which a change will cause.
Additional info in the email cover letter should be helping the reviewer to
understand modifications. Like how much had a backport to change.

Series seems to mostly add a new interface and appears to be tested. Joe, if you
could add some SRU justification into the description part of the bug report.

Thanks,
Stefan

Acked-by: Stefan Bader <[hidden email]>

>
> == Fixes ==
> commit ba7d49b1f0f8e5f24294a880ed576964059af5ef
> Author: Jiri Pirko <[hidden email]>
> Date:   Wed Jan 22 09:05:55 2014 +0100
>
>     rtnetlink: provide api for getting and setting slave info
>
> commit 5d1180fcacc5ceb7da5494acfe9c5e4ebad4f281
> Author: Nicolas Dichtel <[hidden email]>
> Date:   Mon Sep 1 16:07:26 2014 +0200
>
>     rtnl/do_setlink(): set modified when IFLA_TXQLEN is updated
>
> commit 1889b0e7efe8373793069bd3deb7702a51e6f2a5
> Author: Nicolas Dichtel <[hidden email]>
> Date:   Mon Sep 1 16:07:27 2014 +0200
>
>     rtnl/do_setlink(): set modified when IFLA_LINKMODE is updated
>
> commit 90c325e3bfe14ef360de6650fa2a2e92685e5cee
> Author: Nicolas Dichtel <[hidden email]>
> Date:   Mon Sep 1 16:07:28 2014 +0200
>
>     rtnl/do_setlink(): last arg is now a set of flags
>
> commit ba9989069f4e426b1e0ed7018eacc9e1ba607095
> Author: Nicolas Dichtel <[hidden email]>
> Date:   Mon Sep 1 16:07:29 2014 +0200
>
>     rtnl/do_setlink(): notify when a netdev is modified
>
> == Regression Potential ==
> There is a chance for regression due to the number of changes.  However, two of
> the patches are to print warning messages.
>
>
> == Test Case ==
> A test kernel was built with these patches and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.
>
> Jiri Pirko (1):
>   rtnetlink: provide api for getting and setting slave info
>
> Nicolas Dichtel (4):
>   rtnl/do_setlink(): set modified when IFLA_TXQLEN is updated
>   rtnl/do_setlink(): set modified when IFLA_LINKMODE is updated
>   rtnl/do_setlink(): last arg is now a set of flags
>   rtnl/do_setlink(): notify when a netdev is modified
>
>  include/net/rtnetlink.h      |  14 +++
>  include/uapi/linux/if_link.h |   2 +
>  net/core/rtnetlink.c         | 219 +++++++++++++++++++++++++++++++++++--------
>  3 files changed, 195 insertions(+), 40 deletions(-)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Trusty][PATCH 0/5] Fixes for LP:1690094

Thadeu Lima de Souza Cascardo-3
In reply to this post by Joseph Salisbury-3
Acked-by: Thadeu Lima de Souza Cascardo <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU][Trusty][PATCH 0/5] Fixes for LP:1690094

Thadeu Lima de Souza Cascardo-3
In reply to this post by Joseph Salisbury-3
Applied to trusty master-next branch.

Thanks.
Cascardo.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team