* [PATCH net-next 2/2] be2net: Update the driver version to 11.4.0.0
From: Suresh Reddy @ 2017-05-25 2:24 UTC (permalink / raw)
To: netdev; +Cc: Suresh Reddy
In-Reply-To: <1495679079-12025-1-git-send-email-suresh.reddy@broadcom.com>
Signed-off-by: Suresh Reddy <suresh.reddy@broadcom.com>
---
drivers/net/ethernet/emulex/benet/be.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 5056624..674cf9d 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -37,7 +37,7 @@
#include "be_hw.h"
#include "be_roce.h"
-#define DRV_VER "11.1.0.0"
+#define DRV_VER "11.4.0.0"
#define DRV_NAME "be2net"
#define BE_NAME "Emulex BladeEngine2"
#define BE3_NAME "Emulex BladeEngine3"
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/2] be2net: patch-set
From: Suresh Reddy @ 2017-05-25 2:24 UTC (permalink / raw)
To: netdev; +Cc: Suresh Reddy
In-Reply-To: <Suresh.Reddy@broadcom.com>
Hi Dave, Please consider applying these two patches to net-next
Suresh Reddy (2):
be2net: Fix UE detection logic for BE3
be2net: Update the driver version to 11.4.0.0
drivers/net/ethernet/emulex/benet/be.h | 2 +-
drivers/net/ethernet/emulex/benet/be_hw.h | 3 +++
drivers/net/ethernet/emulex/benet/be_main.c | 27 +++++++++++++++++++--------
3 files changed, 23 insertions(+), 9 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 1/2] be2net: Fix UE detection logic for BE3
From: Suresh Reddy @ 2017-05-25 2:24 UTC (permalink / raw)
To: netdev; +Cc: Suresh Reddy
In-Reply-To: <1495679079-12025-1-git-send-email-suresh.reddy@broadcom.com>
On certain platforms BE3 chips may indicate spurious UEs (unrecoverable
error). Because of the UE detection logic was disabled in the driver
for BE3 chips. Because of this, even in cases of a real UE,
a failover will not occur. This patch re-enables UE detection on BE3
and if a UE is detected, reads the POST register. If the POST register,
reports either a FAT_LOG_STATE or a ARMFW_UE, then it means that a valid
UE occurred in the chip.
Signed-off-by: Suresh Reddy <suresh.reddy@broadcom.com>
---
drivers/net/ethernet/emulex/benet/be_hw.h | 3 +++
drivers/net/ethernet/emulex/benet/be_main.c | 27 +++++++++++++++++++--------
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_hw.h b/drivers/net/ethernet/emulex/benet/be_hw.h
index 36e4232..c967f45 100644
--- a/drivers/net/ethernet/emulex/benet/be_hw.h
+++ b/drivers/net/ethernet/emulex/benet/be_hw.h
@@ -49,6 +49,9 @@
#define POST_STAGE_BE_RESET 0x3 /* Host wants to reset chip */
#define POST_STAGE_ARMFW_RDY 0xc000 /* FW is done with POST */
#define POST_STAGE_RECOVERABLE_ERR 0xE000 /* Recoverable err detected */
+/* FW has detected a UE and is dumping FAT log data */
+#define POST_STAGE_FAT_LOG_START 0x0D00
+#define POST_STAGE_ARMFW_UE 0xF000 /*FW has asserted an UE*/
/* Lancer SLIPORT registers */
#define SLIPORT_STATUS_OFFSET 0x404
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index f3a09ab..8000551 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3241,8 +3241,9 @@ void be_detect_error(struct be_adapter *adapter)
{
u32 ue_lo = 0, ue_hi = 0, ue_lo_mask = 0, ue_hi_mask = 0;
u32 sliport_status = 0, sliport_err1 = 0, sliport_err2 = 0;
- u32 i;
struct device *dev = &adapter->pdev->dev;
+ u16 val;
+ u32 i;
if (be_check_error(adapter, BE_ERROR_HW))
return;
@@ -3280,15 +3281,25 @@ void be_detect_error(struct be_adapter *adapter)
ue_lo = (ue_lo & ~ue_lo_mask);
ue_hi = (ue_hi & ~ue_hi_mask);
- /* On certain platforms BE hardware can indicate spurious UEs.
- * Allow HW to stop working completely in case of a real UE.
- * Hence not setting the hw_error for UE detection.
- */
-
if (ue_lo || ue_hi) {
+ /* On certain platforms BE3 hardware can indicate
+ * spurious UEs. In case of a UE in the chip,
+ * the POST register correctly reports either a
+ * FAT_LOG_START state (FW is currently dumping
+ * FAT log data) or a ARMFW_UE state. Check for the
+ * above states to ascertain if the UE is valid or not.
+ */
+ if (BE3_chip(adapter)) {
+ val = be_POST_stage_get(adapter);
+ if ((val & POST_STAGE_FAT_LOG_START)
+ != POST_STAGE_FAT_LOG_START &&
+ (val & POST_STAGE_ARMFW_UE)
+ != POST_STAGE_ARMFW_UE)
+ return;
+ }
+
dev_err(dev, "Error detected in the adapter");
- if (skyhawk_chip(adapter))
- be_set_error(adapter, BE_ERROR_UE);
+ be_set_error(adapter, BE_ERROR_UE);
for (i = 0; ue_lo; ue_lo >>= 1, i++) {
if (ue_lo & 1)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 8/8] net: ipv6: RTM_GETROUTE: return matched fib result when requested
From: David Ahern @ 2017-05-25 2:35 UTC (permalink / raw)
To: Roopa Prabhu, davem; +Cc: netdev, nikolay
In-Reply-To: <1495649951-30417-9-git-send-email-roopa@cumulusnetworks.com>
Since you have to do a v2 ...
On 5/24/17 12:19 PM, Roopa Prabhu wrote:
> @@ -3622,6 +3623,7 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
> memset(&fl6, 0, sizeof(fl6));
> rtm = nlmsg_data(nlh);
> fl6.flowlabel = ip6_make_flowinfo(rtm->rtm_tos, 0);
> + fibmatch = (rtm->rtm_flags & RTM_F_FIB_MATCH) ? true : false;
this is typically done as !!(rtm->rtm_flags & RTM_F_FIB_MATCH)
>
> if (tb[RTA_SRC]) {
> if (nla_len(tb[RTA_SRC]) < sizeof(struct in6_addr))
> @@ -3667,12 +3669,27 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
> if (!ipv6_addr_any(&fl6.saddr))
> flags |= RT6_LOOKUP_F_HAS_SADDR;
>
> - rt = (struct rt6_info *)ip6_route_input_lookup(net, dev, &fl6,
> - flags);
> + if (!fibmatch)
> + rt = (struct rt6_info *)ip6_route_input_lookup(net, dev,
> + &fl6,
> + flags);
> } else {
> fl6.flowi6_oif = oif;
>
> - rt = (struct rt6_info *)ip6_route_output(net, NULL, &fl6);
> + if (!fibmatch)
> + rt = (struct rt6_info *)ip6_route_output_flags(net,
> + NULL,
> + &fl6, 0);
> + }
> +
> + if (fibmatch) {
> + rt = (struct rt6_info *)ip6_route_lookup(net, &fl6, 0);
> + if (rt->dst.error) {
> + err = rt->dst.error;
> + ip6_rt_put(rt);
> + goto errout;
> + }
> +
I'd prefer to see the typecasts go away and use container_of to go from
dst_entry to rt6_info. I realize some of this is movement of existing
code, but better to clean up as we go.
^ permalink raw reply
* Re: [PATCH net-next 1/8] net: ipv4: refactor __ip_route_output_key_hash
From: kbuild test robot @ 2017-05-25 2:38 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: kbuild-all, davem, netdev, dsahern, nikolay
In-Reply-To: <1495649951-30417-2-git-send-email-roopa@cumulusnetworks.com>
[-- Attachment #1: Type: text/plain, Size: 6359 bytes --]
Hi David,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Roopa-Prabhu/net-extend-RTM_GETROUTE-to-return-fib-result/20170525-053253
config: openrisc-or1ksim_defconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 5.4.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
Note: the linux-review/Roopa-Prabhu/net-extend-RTM_GETROUTE-to-return-fib-result/20170525-053253 HEAD 083c4ee9e124d0acf29d159ced8a22cb41665a7a builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
In file included from include/net/route.h:31:0,
from include/net/lwtunnel.h:8,
from include/net/ip_tunnels.h:17,
from include/net/dst_metadata.h:5,
from net/ipv4/route.c:94:
net/ipv4/route.c: In function 'ip_route_output_key_hash_rcu':
>> include/net/ip_fib.h:167:32: error: request for member 'fi' in something not a structure or union
#define FIB_RES_NH(res) ((res).fi->fib_nh[0])
^
include/net/ip_fib.h:196:28: note: in expansion of macro 'FIB_RES_NH'
#define FIB_RES_DEV(res) (FIB_RES_NH(res).nh_dev)
^
net/ipv4/route.c:2399:35: note: in expansion of macro 'FIB_RES_DEV'
dev_out = l3mdev_master_dev_rcu(FIB_RES_DEV(res)) ? :
^
net/ipv4/route.c: In function 'ip_route_output_key_hash':
net/ipv4/route.c:2430:53: error: 'mp_hash' undeclared (first use in this function)
rth = ip_route_output_key_hash_rcu(net, fl4, &res, mp_hash);
^
net/ipv4/route.c:2430:53: note: each undeclared identifier is reported only once for each function it appears in
net/ipv4/route.c:2430:47: error: passing argument 3 of 'ip_route_output_key_hash_rcu' from incompatible pointer type [-Werror=incompatible-pointer-types]
rth = ip_route_output_key_hash_rcu(net, fl4, &res, mp_hash);
^
net/ipv4/route.c:2249:16: note: expected 'const struct sk_buff *' but argument is of type 'struct fib_result *'
struct rtable *ip_route_output_key_hash_rcu(struct net *net, struct flowi4 *fl4,
^
cc1: some warnings being treated as errors
--
In file included from include/net/route.h:31:0,
from include/net/lwtunnel.h:8,
from include/net/ip_tunnels.h:17,
from include/net/dst_metadata.h:5,
from net//ipv4/route.c:94:
net//ipv4/route.c: In function 'ip_route_output_key_hash_rcu':
>> include/net/ip_fib.h:167:32: error: request for member 'fi' in something not a structure or union
#define FIB_RES_NH(res) ((res).fi->fib_nh[0])
^
include/net/ip_fib.h:196:28: note: in expansion of macro 'FIB_RES_NH'
#define FIB_RES_DEV(res) (FIB_RES_NH(res).nh_dev)
^
net//ipv4/route.c:2399:35: note: in expansion of macro 'FIB_RES_DEV'
dev_out = l3mdev_master_dev_rcu(FIB_RES_DEV(res)) ? :
^
net//ipv4/route.c: In function 'ip_route_output_key_hash':
net//ipv4/route.c:2430:53: error: 'mp_hash' undeclared (first use in this function)
rth = ip_route_output_key_hash_rcu(net, fl4, &res, mp_hash);
^
net//ipv4/route.c:2430:53: note: each undeclared identifier is reported only once for each function it appears in
net//ipv4/route.c:2430:47: error: passing argument 3 of 'ip_route_output_key_hash_rcu' from incompatible pointer type [-Werror=incompatible-pointer-types]
rth = ip_route_output_key_hash_rcu(net, fl4, &res, mp_hash);
^
net//ipv4/route.c:2249:16: note: expected 'const struct sk_buff *' but argument is of type 'struct fib_result *'
struct rtable *ip_route_output_key_hash_rcu(struct net *net, struct flowi4 *fl4,
^
cc1: some warnings being treated as errors
vim +/fi +167 include/net/ip_fib.h
5f300893f Thomas Graf 2006-11-09 151 u32 fl_mark;
246955fe4 Robert Olsson 2005-06-20 152 unsigned char fl_tos;
246955fe4 Robert Olsson 2005-06-20 153 unsigned char fl_scope;
246955fe4 Robert Olsson 2005-06-20 154 unsigned char tb_id_in;
246955fe4 Robert Olsson 2005-06-20 155
246955fe4 Robert Olsson 2005-06-20 156 unsigned char tb_id; /* Results */
246955fe4 Robert Olsson 2005-06-20 157 unsigned char prefixlen;
246955fe4 Robert Olsson 2005-06-20 158 unsigned char nh_sel;
246955fe4 Robert Olsson 2005-06-20 159 unsigned char type;
246955fe4 Robert Olsson 2005-06-20 160 unsigned char scope;
246955fe4 Robert Olsson 2005-06-20 161 int err;
246955fe4 Robert Olsson 2005-06-20 162 };
^1da177e4 Linus Torvalds 2005-04-16 163
^1da177e4 Linus Torvalds 2005-04-16 164 #ifdef CONFIG_IP_ROUTE_MULTIPATH
^1da177e4 Linus Torvalds 2005-04-16 165 #define FIB_RES_NH(res) ((res).fi->fib_nh[(res).nh_sel])
^1da177e4 Linus Torvalds 2005-04-16 166 #else /* CONFIG_IP_ROUTE_MULTIPATH */
^1da177e4 Linus Torvalds 2005-04-16 @167 #define FIB_RES_NH(res) ((res).fi->fib_nh[0])
5b9e12dbf Denis V. Lunev 2013-03-13 168 #endif /* CONFIG_IP_ROUTE_MULTIPATH */
^1da177e4 Linus Torvalds 2005-04-16 169
5b9e12dbf Denis V. Lunev 2013-03-13 170 #ifdef CONFIG_IP_MULTIPLE_TABLES
93456b6d7 Denis V. Lunev 2008-01-10 171 #define FIB_TABLE_HASHSZ 256
5b9e12dbf Denis V. Lunev 2013-03-13 172 #else
5b9e12dbf Denis V. Lunev 2013-03-13 173 #define FIB_TABLE_HASHSZ 2
5b9e12dbf Denis V. Lunev 2013-03-13 174 #endif
^1da177e4 Linus Torvalds 2005-04-16 175
:::::: The code at line 167 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7551 bytes --]
^ permalink raw reply
* [PATCH] bonding: Don't update slave->link until ready to commit
From: Nithin Nayak Sujir @ 2017-05-25 2:45 UTC (permalink / raw)
To: davem; +Cc: netdev, Nithin Nayak Sujir, Mahesh Bandewar, Jay Vosburgh
In the loadbalance arp monitoring scheme, when a slave link change is
detected, the slave->link is immediately updated and slave_state_changed
is set. Later down the function, the rtnl_lock is acquired and the
changes are committed, updating the bond link state.
However, the acquisition of the rtnl_lock can fail. The next time the
monitor runs, since slave->link is already updated, it determines that
link is unchanged. This results in the bond link state permanently out
of sync with the slave link.
This patch modifies bond_loadbalance_arp_mon() to handle link changes
identical to bond_ab_arp_{inspect/commit}(). The new link state is
maintained in slave->new_link until we're ready to commit at which point
it's copied into slave->link.
NOTE: miimon_{inspect/commit}() has a more complex state machine
requiring the use of the bond_{propose,commit}_link_state() functions
which maintains the intermediate state in slave->link_new_state. The arp
monitors don't require that.
Testing: This bug is very easy to reproduce with the following steps.
1. In a loop, toggle a slave link of a bond slave interface.
2. In a separate loop, do ifconfig up/down of an unrelated interface to
create contention for rtnl_lock.
Within a few iterations, the bond link goes out of sync with the slave
link.
Signed-off-by: Nithin Nayak Sujir <nsujir@tintri.com>
Cc: Mahesh Bandewar <maheshb@google.com>
Cc: Jay Vosburgh <jay.vosburgh@canonical.com>
---
drivers/net/bonding/bond_main.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7331331..2359478b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2612,11 +2612,13 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
bond_for_each_slave_rcu(bond, slave, iter) {
unsigned long trans_start = dev_trans_start(slave->dev);
+ slave->new_link = BOND_LINK_NOCHANGE;
+
if (slave->link != BOND_LINK_UP) {
if (bond_time_in_interval(bond, trans_start, 1) &&
bond_time_in_interval(bond, slave->last_rx, 1)) {
- slave->link = BOND_LINK_UP;
+ slave->new_link = BOND_LINK_UP;
slave_state_changed = 1;
/* primary_slave has no meaning in round-robin
@@ -2643,7 +2645,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
if (!bond_time_in_interval(bond, trans_start, 2) ||
!bond_time_in_interval(bond, slave->last_rx, 2)) {
- slave->link = BOND_LINK_DOWN;
+ slave->new_link = BOND_LINK_DOWN;
slave_state_changed = 1;
if (slave->link_failure_count < UINT_MAX)
@@ -2674,6 +2676,11 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
if (!rtnl_trylock())
goto re_arm;
+ bond_for_each_slave(bond, slave, iter) {
+ if (slave->new_link != BOND_LINK_NOCHANGE)
+ slave->link = slave->new_link;
+ }
+
if (slave_state_changed) {
bond_slave_state_change(bond);
if (BOND_MODE(bond) == BOND_MODE_XOR)
--
2.8.2
^ permalink raw reply related
* Re: [PATCH net-next 1/8] net: ipv4: refactor __ip_route_output_key_hash
From: Roopa Prabhu @ 2017-05-25 3:05 UTC (permalink / raw)
To: David Ahern
Cc: Rosen, Rami, davem@davemloft.net, netdev@vger.kernel.org,
nikolay@cumulusnetworks.com
In-Reply-To: <692be183-cb75-fe33-605b-e458b10ef240@gmail.com>
On Wed, May 24, 2017 at 6:10 PM, David Ahern <dsahern@gmail.com> wrote:
> On 5/24/17 1:33 PM, Rosen, Rami wrote:
>> Hi, Rupa /David Ahern,
>>
>> First, thanks for this patch set!
>>
>> Second, it seems to me that something might be incorrect here.
>>
>> You have these additions in this patch (1/8):
>> ...
>> +struct rtable *ip_route_output_key_hash_rcu(struct net *net, struct flowi4 *flp,
>> + const struct sk_buff *skb,
>> + struct fib_result *res);
>> ...
>> +struct rtable *ip_route_output_key_hash(struct net *net, struct flowi4 *fl4,
>> + const struct sk_buff *skb)
>> +{
>> + struct fib_result res;
>> + struct rtable *rth;
>> +
>> + res.tclassid = 0;
>> + res.fi = NULL;
>> + res.table = NULL;
>> +
>> + rcu_read_lock();
>> + rth = ip_route_output_key_hash_rcu(net, fl4, &res, mp_hash);
>> rcu_read_unlock();
>> +
>> return rth;
>> }
>> -EXPORT_SYMBOL_GPL(__ip_route_output_key_hash);
>> +EXPORT_SYMBOL_GPL(ip_route_output_key_hash);
>>
>>
>> So the third parameter to ip_route_output_key_hash_rcu() should be skb*, and the fourth parameter should be fib_result *. However, you do not pass the skb parameter
>> when calling ip_route_output_key_hash_rcu() in
>> ip_route_output_key_hash() (in fact you don't use it at all), and you pass mp_hash as the fourth parameter.
>
> Yep, it's a problem with the forward port of the first round of patches.
yep,
>
> Roopa: in include/net/route.h, __ip_route_output_key_hash should be
> removed as well.
ack, thanks Rami and david.
will fix it in v2.
^ permalink raw reply
* [PATCH net-next 0/6] net: another round of extack handling for routing
From: David Ahern @ 2017-05-25 3:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
This set focuses on passing extack through lwtunnel and MPLS with
additional catches for IPv4 route add and minor cleanups in MPLS
encountered passing the extack arg around.
David Ahern (6):
net: ipv4: Add extack message for invalid prefix or length
net: lwtunnel: Add extack to encap attr validation
net: plumb extack arg down to lwtunnel build state
net: Fill in extack for mpls lwt encap
net: mpls: Add extack messages for route add and delete failures
net: mpls: minor cleanups
include/linux/netlink.h | 10 +++
include/net/ip_fib.h | 3 +-
include/net/lwtunnel.h | 22 ++++--
net/core/lwt_bpf.c | 5 +-
net/core/lwtunnel.c | 38 ++++++++--
net/ipv4/fib_frontend.c | 12 +--
net/ipv4/fib_lookup.h | 3 +-
net/ipv4/fib_semantics.c | 20 ++---
net/ipv4/fib_trie.c | 22 ++++--
net/ipv4/ip_tunnel_core.c | 11 ++-
net/ipv6/ila/ila_lwt.c | 5 +-
net/ipv6/route.c | 6 +-
net/ipv6/seg6_iptunnel.c | 5 +-
net/mpls/af_mpls.c | 184 ++++++++++++++++++++++++++++++++++------------
net/mpls/internal.h | 4 +-
net/mpls/mpls_iptunnel.c | 17 +++--
16 files changed, 259 insertions(+), 108 deletions(-)
--
2.11.0 (Apple Git-81)
^ permalink raw reply
* [PATCH net-next 1/6] net: ipv4: Add extack message for invalid prefix or length
From: David Ahern @ 2017-05-25 3:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>
Add extack error message for invalid prefix length and invalid prefix.
Example of the latter is a route spec containing 172.16.100.1/24, where
the /24 mask means the lower 8-bits should be 0. Amazing how easy that
one is to overlook when an EINVAL is returned.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/net/ip_fib.h | 3 ++-
net/ipv4/fib_frontend.c | 6 +++---
net/ipv4/fib_trie.c | 20 +++++++++++++++-----
3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 42e8b8f55f7c..d490a0f8dc4c 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -265,7 +265,8 @@ int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
struct fib_result *res, int fib_flags);
int fib_table_insert(struct net *, struct fib_table *, struct fib_config *,
struct netlink_ext_ack *extack);
-int fib_table_delete(struct net *, struct fib_table *, struct fib_config *);
+int fib_table_delete(struct net *, struct fib_table *, struct fib_config *,
+ struct netlink_ext_ack *extack);
int fib_table_dump(struct fib_table *table, struct sk_buff *skb,
struct netlink_callback *cb);
int fib_table_flush(struct net *net, struct fib_table *table);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 14d2f7bd7c76..2d55882d566e 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -588,7 +588,7 @@ int ip_rt_ioctl(struct net *net, unsigned int cmd, void __user *arg)
if (cmd == SIOCDELRT) {
tb = fib_get_table(net, cfg.fc_table);
if (tb)
- err = fib_table_delete(net, tb, &cfg);
+ err = fib_table_delete(net, tb, &cfg, NULL);
else
err = -ESRCH;
} else {
@@ -732,7 +732,7 @@ static int inet_rtm_delroute(struct sk_buff *skb, struct nlmsghdr *nlh,
goto errout;
}
- err = fib_table_delete(net, tb, &cfg);
+ err = fib_table_delete(net, tb, &cfg, extack);
errout:
return err;
}
@@ -851,7 +851,7 @@ static void fib_magic(int cmd, int type, __be32 dst, int dst_len, struct in_ifad
if (cmd == RTM_NEWROUTE)
fib_table_insert(net, tb, &cfg, NULL);
else
- fib_table_delete(net, tb, &cfg);
+ fib_table_delete(net, tb, &cfg, NULL);
}
void fib_add_ifaddr(struct in_ifaddr *ifa)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 6d0f6c79d9aa..015ff759b0fd 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1115,15 +1115,20 @@ int fib_table_insert(struct net *net, struct fib_table *tb,
u32 key;
int err;
- if (plen > KEYLENGTH)
+ if (plen > KEYLENGTH) {
+ NL_SET_ERR_MSG(extack, "Invalid prefix length");
return -EINVAL;
+ }
key = ntohl(cfg->fc_dst);
pr_debug("Insert table=%u %08x/%d\n", tb->tb_id, key, plen);
- if ((plen < KEYLENGTH) && (key << plen))
+ if ((plen < KEYLENGTH) && (key << plen)) {
+ NL_SET_ERR_MSG(extack,
+ "Invalid prefix for given prefix length");
return -EINVAL;
+ }
fi = fib_create_info(cfg, extack);
if (IS_ERR(fi)) {
@@ -1507,7 +1512,7 @@ static void fib_remove_alias(struct trie *t, struct key_vector *tp,
/* Caller must hold RTNL. */
int fib_table_delete(struct net *net, struct fib_table *tb,
- struct fib_config *cfg)
+ struct fib_config *cfg, struct netlink_ext_ack *extack)
{
struct trie *t = (struct trie *) tb->tb_data;
struct fib_alias *fa, *fa_to_delete;
@@ -1517,13 +1522,18 @@ int fib_table_delete(struct net *net, struct fib_table *tb,
u8 tos = cfg->fc_tos;
u32 key;
- if (plen > KEYLENGTH)
+ if (plen > KEYLENGTH) {
+ NL_SET_ERR_MSG(extack, "Invalid prefix length");
return -EINVAL;
+ }
key = ntohl(cfg->fc_dst);
- if ((plen < KEYLENGTH) && (key << plen))
+ if ((plen < KEYLENGTH) && (key << plen)) {
+ NL_SET_ERR_MSG(extack,
+ "Invalid prefix for given prefix length");
return -EINVAL;
+ }
l = fib_find_node(t, &tp, key);
if (!l)
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* [PATCH net-next 2/6] net: lwtunnel: Add extack to encap attr validation
From: David Ahern @ 2017-05-25 3:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>
Pass extack down to lwtunnel_valid_encap_type and
lwtunnel_valid_encap_type_attr. Add messages for unknown
or unsupported encap types.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/net/lwtunnel.h | 13 +++++++++----
net/core/lwtunnel.c | 18 +++++++++++++-----
net/ipv4/fib_frontend.c | 6 ++++--
net/ipv6/route.c | 4 ++--
4 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index ebfe237aad7e..ca6f002774ef 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -107,8 +107,10 @@ int lwtunnel_encap_add_ops(const struct lwtunnel_encap_ops *op,
unsigned int num);
int lwtunnel_encap_del_ops(const struct lwtunnel_encap_ops *op,
unsigned int num);
-int lwtunnel_valid_encap_type(u16 encap_type);
-int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len);
+int lwtunnel_valid_encap_type(u16 encap_type,
+ struct netlink_ext_ack *extack);
+int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len,
+ struct netlink_ext_ack *extack);
int lwtunnel_build_state(u16 encap_type,
struct nlattr *encap,
unsigned int family, const void *cfg,
@@ -172,11 +174,14 @@ static inline int lwtunnel_encap_del_ops(const struct lwtunnel_encap_ops *op,
return -EOPNOTSUPP;
}
-static inline int lwtunnel_valid_encap_type(u16 encap_type)
+static inline int lwtunnel_valid_encap_type(u16 encap_type,
+ struct netlink_ext_ack *extack)
{
+ NL_SET_ERR_MSG(extack, "CONFIG_LWTUNNEL is not enabled in this kernel");
return -EOPNOTSUPP;
}
-static inline int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len)
+static inline int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len,
+ struct netlink_ext_ack *extack)
{
/* return 0 since we are not walking attr looking for
* RTA_ENCAP_TYPE attribute on nexthops.
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index cfae3d5fe11f..ab840386a74d 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -126,14 +126,16 @@ int lwtunnel_build_state(u16 encap_type,
}
EXPORT_SYMBOL(lwtunnel_build_state);
-int lwtunnel_valid_encap_type(u16 encap_type)
+int lwtunnel_valid_encap_type(u16 encap_type, struct netlink_ext_ack *extack)
{
const struct lwtunnel_encap_ops *ops;
int ret = -EINVAL;
if (encap_type == LWTUNNEL_ENCAP_NONE ||
- encap_type > LWTUNNEL_ENCAP_MAX)
+ encap_type > LWTUNNEL_ENCAP_MAX) {
+ NL_SET_ERR_MSG(extack, "Unknown lwt encapsulation type");
return ret;
+ }
rcu_read_lock();
ops = rcu_dereference(lwtun_encaps[encap_type]);
@@ -153,11 +155,16 @@ int lwtunnel_valid_encap_type(u16 encap_type)
}
}
#endif
- return ops ? 0 : -EOPNOTSUPP;
+ ret = ops ? 0 : -EOPNOTSUPP;
+ if (ret < 0)
+ NL_SET_ERR_MSG(extack, "lwt encapsulation type not supported");
+
+ return ret;
}
EXPORT_SYMBOL(lwtunnel_valid_encap_type);
-int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int remaining)
+int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int remaining,
+ struct netlink_ext_ack *extack)
{
struct rtnexthop *rtnh = (struct rtnexthop *)attr;
struct nlattr *nla_entype;
@@ -174,7 +181,8 @@ int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int remaining)
if (nla_entype) {
encap_type = nla_get_u16(nla_entype);
- if (lwtunnel_valid_encap_type(encap_type) != 0)
+ if (lwtunnel_valid_encap_type(encap_type,
+ extack) != 0)
return -EOPNOTSUPP;
}
}
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 2d55882d566e..f03fe8521f6e 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -684,7 +684,8 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
break;
case RTA_MULTIPATH:
err = lwtunnel_valid_encap_type_attr(nla_data(attr),
- nla_len(attr));
+ nla_len(attr),
+ extack);
if (err < 0)
goto errout;
cfg->fc_mp = nla_data(attr);
@@ -701,7 +702,8 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
break;
case RTA_ENCAP_TYPE:
cfg->fc_encap_type = nla_get_u16(attr);
- err = lwtunnel_valid_encap_type(cfg->fc_encap_type);
+ err = lwtunnel_valid_encap_type(cfg->fc_encap_type,
+ extack);
if (err < 0)
goto errout;
break;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 80bda31ffbbe..afc76b24ee6b 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3016,7 +3016,7 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
cfg->fc_mp_len = nla_len(tb[RTA_MULTIPATH]);
err = lwtunnel_valid_encap_type_attr(cfg->fc_mp,
- cfg->fc_mp_len);
+ cfg->fc_mp_len, extack);
if (err < 0)
goto errout;
}
@@ -3035,7 +3035,7 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
if (tb[RTA_ENCAP_TYPE]) {
cfg->fc_encap_type = nla_get_u16(tb[RTA_ENCAP_TYPE]);
- err = lwtunnel_valid_encap_type(cfg->fc_encap_type);
+ err = lwtunnel_valid_encap_type(cfg->fc_encap_type, extack);
if (err < 0)
goto errout;
}
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* [PATCH net-next 3/6] net: plumb extack arg down to lwtunnel build state
From: David Ahern @ 2017-05-25 3:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>
Pass extack arg down to lwtunnel_build_state and the build_state callbacks.
Add messages for failures in lwtunnel_build_state, and add the extarg to
nla_parse where possible in the build_state callbacks.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/linux/netlink.h | 10 ++++++++++
include/net/lwtunnel.h | 9 ++++++---
net/core/lwt_bpf.c | 5 +++--
net/core/lwtunnel.c | 20 +++++++++++++++++---
net/ipv4/fib_lookup.h | 3 ++-
net/ipv4/fib_semantics.c | 20 +++++++++++---------
net/ipv4/fib_trie.c | 2 +-
net/ipv4/ip_tunnel_core.c | 11 +++++++----
net/ipv6/ila/ila_lwt.c | 5 +++--
net/ipv6/route.c | 2 +-
net/ipv6/seg6_iptunnel.c | 5 +++--
net/mpls/mpls_iptunnel.c | 5 +++--
12 files changed, 67 insertions(+), 30 deletions(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index a68aad484c69..8664fd26eb5d 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -102,6 +102,16 @@ struct netlink_ext_ack {
(extack)->bad_attr = (attr); \
} while (0)
+#define NL_SET_ERR_MSG_ATTR(extack, attr, msg) do { \
+ static const char __msg[] = (msg); \
+ struct netlink_ext_ack *__extack = (extack); \
+ \
+ if (__extack) { \
+ __extack->_msg = __msg; \
+ __extack->bad_attr = (attr); \
+ } \
+} while (0)
+
extern void netlink_kernel_release(struct sock *sk);
extern int __netlink_change_ngroups(struct sock *sk, unsigned int groups);
extern int netlink_change_ngroups(struct sock *sk, unsigned int groups);
diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index ca6f002774ef..7c26863b8cf4 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -35,7 +35,8 @@ struct lwtunnel_state {
struct lwtunnel_encap_ops {
int (*build_state)(struct nlattr *encap,
unsigned int family, const void *cfg,
- struct lwtunnel_state **ts);
+ struct lwtunnel_state **ts,
+ struct netlink_ext_ack *extack);
void (*destroy_state)(struct lwtunnel_state *lws);
int (*output)(struct net *net, struct sock *sk, struct sk_buff *skb);
int (*input)(struct sk_buff *skb);
@@ -114,7 +115,8 @@ int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len,
int lwtunnel_build_state(u16 encap_type,
struct nlattr *encap,
unsigned int family, const void *cfg,
- struct lwtunnel_state **lws);
+ struct lwtunnel_state **lws,
+ struct netlink_ext_ack *extack);
int lwtunnel_fill_encap(struct sk_buff *skb,
struct lwtunnel_state *lwtstate);
int lwtunnel_get_encap_size(struct lwtunnel_state *lwtstate);
@@ -192,7 +194,8 @@ static inline int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len,
static inline int lwtunnel_build_state(u16 encap_type,
struct nlattr *encap,
unsigned int family, const void *cfg,
- struct lwtunnel_state **lws)
+ struct lwtunnel_state **lws,
+ struct netlink_ext_ack *extack)
{
return -EOPNOTSUPP;
}
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index b3bc0a31af9f..1307731ddfe4 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -240,7 +240,8 @@ static const struct nla_policy bpf_nl_policy[LWT_BPF_MAX + 1] = {
static int bpf_build_state(struct nlattr *nla,
unsigned int family, const void *cfg,
- struct lwtunnel_state **ts)
+ struct lwtunnel_state **ts,
+ struct netlink_ext_ack *extack)
{
struct nlattr *tb[LWT_BPF_MAX + 1];
struct lwtunnel_state *newts;
@@ -250,7 +251,7 @@ static int bpf_build_state(struct nlattr *nla,
if (family != AF_INET && family != AF_INET6)
return -EAFNOSUPPORT;
- ret = nla_parse_nested(tb, LWT_BPF_MAX, nla, bpf_nl_policy, NULL);
+ ret = nla_parse_nested(tb, LWT_BPF_MAX, nla, bpf_nl_policy, extack);
if (ret < 0)
return ret;
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index ab840386a74d..a0feca1df7b5 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -103,25 +103,39 @@ EXPORT_SYMBOL(lwtunnel_encap_del_ops);
int lwtunnel_build_state(u16 encap_type,
struct nlattr *encap, unsigned int family,
- const void *cfg, struct lwtunnel_state **lws)
+ const void *cfg, struct lwtunnel_state **lws,
+ struct netlink_ext_ack *extack)
{
const struct lwtunnel_encap_ops *ops;
+ bool found = false;
int ret = -EINVAL;
if (encap_type == LWTUNNEL_ENCAP_NONE ||
- encap_type > LWTUNNEL_ENCAP_MAX)
+ encap_type > LWTUNNEL_ENCAP_MAX) {
+ NL_SET_BAD_ATTR(extack, encap);
+ NL_SET_ERR_MSG(extack, "Unknown LWT encapsulation type");
return ret;
+ }
ret = -EOPNOTSUPP;
rcu_read_lock();
ops = rcu_dereference(lwtun_encaps[encap_type]);
if (likely(ops && ops->build_state && try_module_get(ops->owner))) {
- ret = ops->build_state(encap, family, cfg, lws);
+ found = true;
+ ret = ops->build_state(encap, family, cfg, lws, extack);
if (ret)
module_put(ops->owner);
}
rcu_read_unlock();
+ /* don't rely on -EOPNOTSUPP to detect match as build_state
+ * handlers could return it
+ */
+ if (!found) {
+ NL_SET_BAD_ATTR(extack, encap);
+ NL_SET_ERR_MSG(extack, "LWT encapsulation type not supported");
+ }
+
return ret;
}
EXPORT_SYMBOL(lwtunnel_build_state);
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index 2704e08545da..769ab87ebc4b 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -30,7 +30,8 @@ static inline void fib_alias_accessed(struct fib_alias *fa)
void fib_release_info(struct fib_info *);
struct fib_info *fib_create_info(struct fib_config *cfg,
struct netlink_ext_ack *extack);
-int fib_nh_match(struct fib_config *cfg, struct fib_info *fi);
+int fib_nh_match(struct fib_config *cfg, struct fib_info *fi,
+ struct netlink_ext_ack *extack);
int fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event, u32 tb_id,
u8 type, __be32 dst, int dst_len, u8 tos, struct fib_info *fi,
unsigned int);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 4852e183afe0..89e8fcc8449b 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -530,7 +530,7 @@ static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
ret = lwtunnel_build_state(nla_get_u16(
nla_entype),
nla, AF_INET, cfg,
- &lwtstate);
+ &lwtstate, extack);
if (ret)
goto errout;
nexthop_nh->nh_lwtstate =
@@ -612,7 +612,8 @@ static inline void fib_add_weight(struct fib_info *fi,
static int fib_encap_match(u16 encap_type,
struct nlattr *encap,
const struct fib_nh *nh,
- const struct fib_config *cfg)
+ const struct fib_config *cfg,
+ struct netlink_ext_ack *extack)
{
struct lwtunnel_state *lwtstate;
int ret, result = 0;
@@ -620,8 +621,8 @@ static int fib_encap_match(u16 encap_type,
if (encap_type == LWTUNNEL_ENCAP_NONE)
return 0;
- ret = lwtunnel_build_state(encap_type, encap,
- AF_INET, cfg, &lwtstate);
+ ret = lwtunnel_build_state(encap_type, encap, AF_INET,
+ cfg, &lwtstate, extack);
if (!ret) {
result = lwtunnel_cmp_encap(lwtstate, nh->nh_lwtstate);
lwtstate_free(lwtstate);
@@ -630,7 +631,8 @@ static int fib_encap_match(u16 encap_type,
return result;
}
-int fib_nh_match(struct fib_config *cfg, struct fib_info *fi)
+int fib_nh_match(struct fib_config *cfg, struct fib_info *fi,
+ struct netlink_ext_ack *extack)
{
#ifdef CONFIG_IP_ROUTE_MULTIPATH
struct rtnexthop *rtnh;
@@ -642,9 +644,9 @@ int fib_nh_match(struct fib_config *cfg, struct fib_info *fi)
if (cfg->fc_oif || cfg->fc_gw) {
if (cfg->fc_encap) {
- if (fib_encap_match(cfg->fc_encap_type,
- cfg->fc_encap, fi->fib_nh, cfg))
- return 1;
+ if (fib_encap_match(cfg->fc_encap_type, cfg->fc_encap,
+ fi->fib_nh, cfg, extack))
+ return 1;
}
if ((!cfg->fc_oif || cfg->fc_oif == fi->fib_nh->nh_oif) &&
(!cfg->fc_gw || cfg->fc_gw == fi->fib_nh->nh_gw))
@@ -1145,7 +1147,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg,
}
err = lwtunnel_build_state(cfg->fc_encap_type,
cfg->fc_encap, AF_INET, cfg,
- &lwtstate);
+ &lwtstate, extack);
if (err)
goto failure;
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 015ff759b0fd..03800e39e924 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1561,7 +1561,7 @@ int fib_table_delete(struct net *net, struct fib_table *tb,
fi->fib_prefsrc == cfg->fc_prefsrc) &&
(!cfg->fc_protocol ||
fi->fib_protocol == cfg->fc_protocol) &&
- fib_nh_match(cfg, fi) == 0) {
+ fib_nh_match(cfg, fi, extack) == 0) {
fa_to_delete = fa;
break;
}
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index baf196eaf1d8..90e11479c725 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -228,14 +228,16 @@ static const struct nla_policy ip_tun_policy[LWTUNNEL_IP_MAX + 1] = {
static int ip_tun_build_state(struct nlattr *attr,
unsigned int family, const void *cfg,
- struct lwtunnel_state **ts)
+ struct lwtunnel_state **ts,
+ struct netlink_ext_ack *extack)
{
struct ip_tunnel_info *tun_info;
struct lwtunnel_state *new_state;
struct nlattr *tb[LWTUNNEL_IP_MAX + 1];
int err;
- err = nla_parse_nested(tb, LWTUNNEL_IP_MAX, attr, ip_tun_policy, NULL);
+ err = nla_parse_nested(tb, LWTUNNEL_IP_MAX, attr, ip_tun_policy,
+ extack);
if (err < 0)
return err;
@@ -325,7 +327,8 @@ static const struct nla_policy ip6_tun_policy[LWTUNNEL_IP6_MAX + 1] = {
static int ip6_tun_build_state(struct nlattr *attr,
unsigned int family, const void *cfg,
- struct lwtunnel_state **ts)
+ struct lwtunnel_state **ts,
+ struct netlink_ext_ack *extack)
{
struct ip_tunnel_info *tun_info;
struct lwtunnel_state *new_state;
@@ -333,7 +336,7 @@ static int ip6_tun_build_state(struct nlattr *attr,
int err;
err = nla_parse_nested(tb, LWTUNNEL_IP6_MAX, attr, ip6_tun_policy,
- NULL);
+ extack);
if (err < 0)
return err;
diff --git a/net/ipv6/ila/ila_lwt.c b/net/ipv6/ila/ila_lwt.c
index f4a413aba423..0c02a09bc351 100644
--- a/net/ipv6/ila/ila_lwt.c
+++ b/net/ipv6/ila/ila_lwt.c
@@ -117,7 +117,8 @@ static const struct nla_policy ila_nl_policy[ILA_ATTR_MAX + 1] = {
static int ila_build_state(struct nlattr *nla,
unsigned int family, const void *cfg,
- struct lwtunnel_state **ts)
+ struct lwtunnel_state **ts,
+ struct netlink_ext_ack *extack)
{
struct ila_lwt *ilwt;
struct ila_params *p;
@@ -146,7 +147,7 @@ static int ila_build_state(struct nlattr *nla,
return -EINVAL;
}
- ret = nla_parse_nested(tb, ILA_ATTR_MAX, nla, ila_nl_policy, NULL);
+ ret = nla_parse_nested(tb, ILA_ATTR_MAX, nla, ila_nl_policy, extack);
if (ret < 0)
return ret;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index afc76b24ee6b..0146116f8539 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1939,7 +1939,7 @@ static struct rt6_info *ip6_route_info_create(struct fib6_config *cfg,
err = lwtunnel_build_state(cfg->fc_encap_type,
cfg->fc_encap, AF_INET6, cfg,
- &lwtstate);
+ &lwtstate, extack);
if (err)
goto out;
rt->dst.lwtstate = lwtstate_get(lwtstate);
diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 6a495490d43e..264d772d3c7d 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -326,7 +326,8 @@ static int seg6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
static int seg6_build_state(struct nlattr *nla,
unsigned int family, const void *cfg,
- struct lwtunnel_state **ts)
+ struct lwtunnel_state **ts,
+ struct netlink_ext_ack *extack)
{
struct nlattr *tb[SEG6_IPTUNNEL_MAX + 1];
struct seg6_iptunnel_encap *tuninfo;
@@ -336,7 +337,7 @@ static int seg6_build_state(struct nlattr *nla,
int err;
err = nla_parse_nested(tb, SEG6_IPTUNNEL_MAX, nla,
- seg6_iptunnel_policy, NULL);
+ seg6_iptunnel_policy, extack);
if (err < 0)
return err;
diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index 369c7a23c86c..15e1aa708e50 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -159,7 +159,8 @@ static int mpls_xmit(struct sk_buff *skb)
static int mpls_build_state(struct nlattr *nla,
unsigned int family, const void *cfg,
- struct lwtunnel_state **ts)
+ struct lwtunnel_state **ts,
+ struct netlink_ext_ack *extack)
{
struct mpls_iptunnel_encap *tun_encap_info;
struct nlattr *tb[MPLS_IPTUNNEL_MAX + 1];
@@ -168,7 +169,7 @@ static int mpls_build_state(struct nlattr *nla,
int ret;
ret = nla_parse_nested(tb, MPLS_IPTUNNEL_MAX, nla,
- mpls_iptunnel_policy, NULL);
+ mpls_iptunnel_policy, extack);
if (ret < 0)
return ret;
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* [PATCH net-next 4/6] net: Fill in extack for mpls lwt encap
From: David Ahern @ 2017-05-25 3:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>
Fill in extack for errors in build_state for mpls lwt encap including
passing extack to nla_get_labels and adding error messages for failures
in it.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/mpls/af_mpls.c | 49 ++++++++++++++++++++++++++++++++++++++----------
net/mpls/internal.h | 2 +-
net/mpls/mpls_iptunnel.c | 12 +++++++-----
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 257ec66009da..f3830951fb1c 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -728,8 +728,8 @@ static int mpls_nh_build(struct net *net, struct mpls_route *rt,
goto errout;
if (newdst) {
- err = nla_get_labels(newdst, max_labels,
- &nh->nh_labels, nh->nh_label);
+ err = nla_get_labels(newdst, max_labels, &nh->nh_labels,
+ nh->nh_label, NULL);
if (err)
goto errout;
}
@@ -782,7 +782,8 @@ static u8 mpls_count_nexthops(struct rtnexthop *rtnh, int len,
nla = nla_find(attrs, attrlen, RTA_NEWDST);
if (nla &&
- nla_get_labels(nla, MAX_NEW_LABELS, &n_labels, NULL) != 0)
+ nla_get_labels(nla, MAX_NEW_LABELS, &n_labels,
+ NULL, NULL) != 0)
return 0;
*max_labels = max_t(u8, *max_labels, n_labels);
@@ -1541,8 +1542,8 @@ int nla_put_labels(struct sk_buff *skb, int attrtype,
}
EXPORT_SYMBOL_GPL(nla_put_labels);
-int nla_get_labels(const struct nlattr *nla,
- u8 max_labels, u8 *labels, u32 label[])
+int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
+ u32 label[], struct netlink_ext_ack *extack)
{
unsigned len = nla_len(nla);
struct mpls_shim_hdr *nla_label;
@@ -1553,13 +1554,18 @@ int nla_get_labels(const struct nlattr *nla,
/* len needs to be an even multiple of 4 (the label size). Number
* of labels is a u8 so check for overflow.
*/
- if (len & 3 || len / 4 > 255)
+ if (len & 3 || len / 4 > 255) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Invalid length for labels attribute");
return -EINVAL;
+ }
/* Limit the number of new labels allowed */
nla_labels = len/4;
- if (nla_labels > max_labels)
+ if (nla_labels > max_labels) {
+ NL_SET_ERR_MSG(extack, "Too many labels");
return -EINVAL;
+ }
/* when label == NULL, caller wants number of labels */
if (!label)
@@ -1574,8 +1580,29 @@ int nla_get_labels(const struct nlattr *nla,
/* Ensure the bottom of stack flag is properly set
* and ttl and tc are both clear.
*/
- if ((dec.bos != bos) || dec.ttl || dec.tc)
+ if (dec.ttl) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "TTL in label must be 0");
+ return -EINVAL;
+ }
+
+ if (dec.tc) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Traffic class in label must be 0");
return -EINVAL;
+ }
+
+ if (dec.bos != bos) {
+ NL_SET_BAD_ATTR(extack, nla);
+ if (bos) {
+ NL_SET_ERR_MSG(extack,
+ "BOS bit must be set in first label");
+ } else {
+ NL_SET_ERR_MSG(extack,
+ "BOS bit can only be set in first label");
+ }
+ return -EINVAL;
+ }
switch (dec.label) {
case MPLS_LABEL_IMPLNULL:
@@ -1583,6 +1610,8 @@ int nla_get_labels(const struct nlattr *nla,
* assign and distribute, but which never
* actually appears in the encapsulation.
*/
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Implicit NULL Label (3) can not be used in encapsulation");
return -EINVAL;
}
@@ -1696,14 +1725,14 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh,
case RTA_NEWDST:
if (nla_get_labels(nla, MAX_NEW_LABELS,
&cfg->rc_output_labels,
- cfg->rc_output_label))
+ cfg->rc_output_label, NULL))
goto errout;
break;
case RTA_DST:
{
u8 label_count;
if (nla_get_labels(nla, 1, &label_count,
- &cfg->rc_label))
+ &cfg->rc_label, NULL))
goto errout;
/* Reserved labels may not be set */
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index 4db6a5971322..e59f299ceb3f 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -203,7 +203,7 @@ static inline struct mpls_dev *mpls_dev_get(const struct net_device *dev)
int nla_put_labels(struct sk_buff *skb, int attrtype, u8 labels,
const u32 label[]);
int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
- u32 label[]);
+ u32 label[], struct netlink_ext_ack *extack);
int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
u8 via[]);
bool mpls_output_possible(const struct net_device *dev);
diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index 15e1aa708e50..6e558a419f60 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -173,13 +173,14 @@ static int mpls_build_state(struct nlattr *nla,
if (ret < 0)
return ret;
- if (!tb[MPLS_IPTUNNEL_DST])
+ if (!tb[MPLS_IPTUNNEL_DST]) {
+ NL_SET_ERR_MSG(extack, "MPLS_IPTUNNEL_DST attribute is missing");
return -EINVAL;
-
+ }
/* determine number of labels */
- if (nla_get_labels(tb[MPLS_IPTUNNEL_DST],
- MAX_NEW_LABELS, &n_labels, NULL))
+ if (nla_get_labels(tb[MPLS_IPTUNNEL_DST], MAX_NEW_LABELS,
+ &n_labels, NULL, extack))
return -EINVAL;
newts = lwtunnel_state_alloc(sizeof(*tun_encap_info) +
@@ -189,7 +190,8 @@ static int mpls_build_state(struct nlattr *nla,
tun_encap_info = mpls_lwtunnel_encap(newts);
ret = nla_get_labels(tb[MPLS_IPTUNNEL_DST], n_labels,
- &tun_encap_info->labels, tun_encap_info->label);
+ &tun_encap_info->labels, tun_encap_info->label,
+ extack);
if (ret)
goto errout;
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* [PATCH net-next 5/6] net: mpls: Add extack messages for route add and delete failures
From: David Ahern @ 2017-05-25 3:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>
Add error messages for failures in adding and deleting mpls routes.
This covers most of the annoying EINVAL errors.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/mpls/af_mpls.c | 136 +++++++++++++++++++++++++++++++++++++---------------
net/mpls/internal.h | 2 +-
2 files changed, 98 insertions(+), 40 deletions(-)
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index f3830951fb1c..d0120f8c8a2c 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -720,7 +720,8 @@ static int mpls_nh_build_from_cfg(struct mpls_route_config *cfg,
static int mpls_nh_build(struct net *net, struct mpls_route *rt,
struct mpls_nh *nh, int oif, struct nlattr *via,
- struct nlattr *newdst, u8 max_labels)
+ struct nlattr *newdst, u8 max_labels,
+ struct netlink_ext_ack *extack)
{
int err = -ENOMEM;
@@ -729,14 +730,14 @@ static int mpls_nh_build(struct net *net, struct mpls_route *rt,
if (newdst) {
err = nla_get_labels(newdst, max_labels, &nh->nh_labels,
- nh->nh_label, NULL);
+ nh->nh_label, extack);
if (err)
goto errout;
}
if (via) {
err = nla_get_via(via, &nh->nh_via_alen, &nh->nh_via_table,
- __mpls_nh_via(rt, nh));
+ __mpls_nh_via(rt, nh), extack);
if (err)
goto errout;
} else {
@@ -803,7 +804,8 @@ static u8 mpls_count_nexthops(struct rtnexthop *rtnh, int len,
}
static int mpls_nh_build_multi(struct mpls_route_config *cfg,
- struct mpls_route *rt, u8 max_labels)
+ struct mpls_route *rt, u8 max_labels,
+ struct netlink_ext_ack *extack)
{
struct rtnexthop *rtnh = cfg->rc_mp;
struct nlattr *nla_via, *nla_newdst;
@@ -837,7 +839,7 @@ static int mpls_nh_build_multi(struct mpls_route_config *cfg,
err = mpls_nh_build(cfg->rc_nlinfo.nl_net, rt, nh,
rtnh->rtnh_ifindex, nla_via, nla_newdst,
- max_labels);
+ max_labels, extack);
if (err)
goto errout;
@@ -856,7 +858,8 @@ static int mpls_nh_build_multi(struct mpls_route_config *cfg,
return err;
}
-static int mpls_route_add(struct mpls_route_config *cfg)
+static int mpls_route_add(struct mpls_route_config *cfg,
+ struct netlink_ext_ack *extack)
{
struct mpls_route __rcu **platform_label;
struct net *net = cfg->rc_nlinfo.nl_net;
@@ -876,17 +879,25 @@ static int mpls_route_add(struct mpls_route_config *cfg)
}
/* Reserved labels may not be set */
- if (index < MPLS_LABEL_FIRST_UNRESERVED)
+ if (index < MPLS_LABEL_FIRST_UNRESERVED) {
+ NL_SET_ERR_MSG(extack,
+ "Invalid label - must be MPLS_LABEL_FIRST_UNRESERVED or higher");
goto errout;
+ }
/* The full 20 bit range may not be supported. */
- if (index >= net->mpls.platform_labels)
+ if (index >= net->mpls.platform_labels) {
+ NL_SET_ERR_MSG(extack,
+ "Label >= configured max in platform_labels");
goto errout;
+ }
/* Append makes no sense with mpls */
err = -EOPNOTSUPP;
- if (cfg->rc_nlflags & NLM_F_APPEND)
+ if (cfg->rc_nlflags & NLM_F_APPEND) {
+ NL_SET_ERR_MSG(extack, "MPLS does not support route append");
goto errout;
+ }
err = -EEXIST;
platform_label = rtnl_dereference(net->mpls.platform_label);
@@ -913,8 +924,10 @@ static int mpls_route_add(struct mpls_route_config *cfg)
nhs = 1;
}
- if (nhs == 0)
+ if (nhs == 0) {
+ NL_SET_ERR_MSG(extack, "Route does not contain a nexthop");
goto errout;
+ }
err = -ENOMEM;
rt = mpls_rt_alloc(nhs, max_via_alen, max_labels);
@@ -928,7 +941,7 @@ static int mpls_route_add(struct mpls_route_config *cfg)
rt->rt_ttl_propagate = cfg->rc_ttl_propagate;
if (cfg->rc_mp)
- err = mpls_nh_build_multi(cfg, rt, max_labels);
+ err = mpls_nh_build_multi(cfg, rt, max_labels, extack);
else
err = mpls_nh_build_from_cfg(cfg, rt);
if (err)
@@ -944,7 +957,8 @@ static int mpls_route_add(struct mpls_route_config *cfg)
return err;
}
-static int mpls_route_del(struct mpls_route_config *cfg)
+static int mpls_route_del(struct mpls_route_config *cfg,
+ struct netlink_ext_ack *extack)
{
struct net *net = cfg->rc_nlinfo.nl_net;
unsigned index;
@@ -953,12 +967,18 @@ static int mpls_route_del(struct mpls_route_config *cfg)
index = cfg->rc_label;
/* Reserved labels may not be removed */
- if (index < MPLS_LABEL_FIRST_UNRESERVED)
+ if (index < MPLS_LABEL_FIRST_UNRESERVED) {
+ NL_SET_ERR_MSG(extack,
+ "Invalid label - must be MPLS_LABEL_FIRST_UNRESERVED or higher");
goto errout;
+ }
/* The full 20 bit range may not be supported */
- if (index >= net->mpls.platform_labels)
+ if (index >= net->mpls.platform_labels) {
+ NL_SET_ERR_MSG(extack,
+ "Label >= configured max in platform_labels");
goto errout;
+ }
mpls_route_update(net, index, NULL, &cfg->rc_nlinfo);
@@ -1623,19 +1643,25 @@ int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
}
EXPORT_SYMBOL_GPL(nla_get_labels);
-int nla_get_via(const struct nlattr *nla, u8 *via_alen,
- u8 *via_table, u8 via_addr[])
+int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
+ u8 via_addr[], struct netlink_ext_ack *extack)
{
struct rtvia *via = nla_data(nla);
int err = -EINVAL;
int alen;
- if (nla_len(nla) < offsetof(struct rtvia, rtvia_addr))
+ if (nla_len(nla) < offsetof(struct rtvia, rtvia_addr)) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Invalid attribute length for RTA_VIA");
goto errout;
+ }
alen = nla_len(nla) -
offsetof(struct rtvia, rtvia_addr);
- if (alen > MAX_VIA_ALEN)
+ if (alen > MAX_VIA_ALEN) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Invalid address length for RTA_VIA");
goto errout;
+ }
/* Validate the address family */
switch (via->rtvia_family) {
@@ -1665,8 +1691,10 @@ int nla_get_via(const struct nlattr *nla, u8 *via_alen,
return err;
}
-static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh,
- struct mpls_route_config *cfg)
+static int rtm_to_route_config(struct sk_buff *skb,
+ struct nlmsghdr *nlh,
+ struct mpls_route_config *cfg,
+ struct netlink_ext_ack *extack)
{
struct rtmsg *rtm;
struct nlattr *tb[RTA_MAX+1];
@@ -1674,35 +1702,54 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh,
int err;
err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_mpls_policy,
- NULL);
+ extack);
if (err < 0)
goto errout;
err = -EINVAL;
rtm = nlmsg_data(nlh);
- if (rtm->rtm_family != AF_MPLS)
+ if (rtm->rtm_family != AF_MPLS) {
+ NL_SET_ERR_MSG(extack, "Invalid address family in rtmsg");
goto errout;
- if (rtm->rtm_dst_len != 20)
+ }
+ if (rtm->rtm_dst_len != 20) {
+ NL_SET_ERR_MSG(extack, "rtm_dst_len must be 20 for MPLS");
goto errout;
- if (rtm->rtm_src_len != 0)
+ }
+ if (rtm->rtm_src_len != 0) {
+ NL_SET_ERR_MSG(extack, "rtm_src_len must be 0 for MPLS");
goto errout;
- if (rtm->rtm_tos != 0)
+ }
+ if (rtm->rtm_tos != 0) {
+ NL_SET_ERR_MSG(extack, "rtm_tos must be 0 for MPLS");
goto errout;
- if (rtm->rtm_table != RT_TABLE_MAIN)
+ }
+ if (rtm->rtm_table != RT_TABLE_MAIN) {
+ NL_SET_ERR_MSG(extack,
+ "MPLS only supports the main route table");
goto errout;
+ }
/* Any value is acceptable for rtm_protocol */
/* As mpls uses destination specific addresses
* (or source specific address in the case of multicast)
* all addresses have universal scope.
*/
- if (rtm->rtm_scope != RT_SCOPE_UNIVERSE)
+ if (rtm->rtm_scope != RT_SCOPE_UNIVERSE) {
+ NL_SET_ERR_MSG(extack,
+ "Invalid route scope - MPLS only supports UNIVERSE");
goto errout;
- if (rtm->rtm_type != RTN_UNICAST)
+ }
+ if (rtm->rtm_type != RTN_UNICAST) {
+ NL_SET_ERR_MSG(extack,
+ "Invalid route type - MPLS only supports UNICAST");
goto errout;
- if (rtm->rtm_flags != 0)
+ }
+ if (rtm->rtm_flags != 0) {
+ NL_SET_ERR_MSG(extack, "rtm_flags must be 0 for MPLS");
goto errout;
+ }
cfg->rc_label = LABEL_NOT_SPECIFIED;
cfg->rc_protocol = rtm->rtm_protocol;
@@ -1725,26 +1772,33 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh,
case RTA_NEWDST:
if (nla_get_labels(nla, MAX_NEW_LABELS,
&cfg->rc_output_labels,
- cfg->rc_output_label, NULL))
+ cfg->rc_output_label, extack)) {
+ NL_SET_BAD_ATTR(extack, nla);
goto errout;
+ }
break;
case RTA_DST:
{
u8 label_count;
if (nla_get_labels(nla, 1, &label_count,
- &cfg->rc_label, NULL))
+ &cfg->rc_label, extack)) {
+ NL_SET_BAD_ATTR(extack, nla);
goto errout;
+ }
/* Reserved labels may not be set */
- if (cfg->rc_label < MPLS_LABEL_FIRST_UNRESERVED)
+ if (cfg->rc_label < MPLS_LABEL_FIRST_UNRESERVED) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Invalid label for RTA_DST");
goto errout;
-
+ }
break;
}
case RTA_VIA:
{
if (nla_get_via(nla, &cfg->rc_via_alen,
- &cfg->rc_via_table, cfg->rc_via))
+ &cfg->rc_via_table, cfg->rc_via,
+ extack))
goto errout;
break;
}
@@ -1758,14 +1812,18 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh,
{
u8 ttl_propagate = nla_get_u8(nla);
- if (ttl_propagate > 1)
+ if (ttl_propagate > 1) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "RTA_TTL_PROPAGATE can only be 0 or 1");
goto errout;
+ }
cfg->rc_ttl_propagate = ttl_propagate ?
MPLS_TTL_PROP_ENABLED :
MPLS_TTL_PROP_DISABLED;
break;
}
default:
+ NL_SET_ERR_MSG_ATTR(extack, nla, "Unknown attribute");
/* Unsupported attribute */
goto errout;
}
@@ -1786,11 +1844,11 @@ static int mpls_rtm_delroute(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!cfg)
return -ENOMEM;
- err = rtm_to_route_config(skb, nlh, cfg);
+ err = rtm_to_route_config(skb, nlh, cfg, extack);
if (err < 0)
goto out;
- err = mpls_route_del(cfg);
+ err = mpls_route_del(cfg, extack);
out:
kfree(cfg);
@@ -1808,11 +1866,11 @@ static int mpls_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!cfg)
return -ENOMEM;
- err = rtm_to_route_config(skb, nlh, cfg);
+ err = rtm_to_route_config(skb, nlh, cfg, extack);
if (err < 0)
goto out;
- err = mpls_route_add(cfg);
+ err = mpls_route_add(cfg, extack);
out:
kfree(cfg);
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index e59f299ceb3f..a015a6a1143b 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -205,7 +205,7 @@ int nla_put_labels(struct sk_buff *skb, int attrtype, u8 labels,
int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
u32 label[], struct netlink_ext_ack *extack);
int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
- u8 via[]);
+ u8 via[], struct netlink_ext_ack *extack);
bool mpls_output_possible(const struct net_device *dev);
unsigned int mpls_dev_mtu(const struct net_device *dev);
bool mpls_pkt_too_big(const struct sk_buff *skb, unsigned int mtu);
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* [PATCH net-next 6/6] net: mpls: minor cleanups
From: David Ahern @ 2017-05-25 3:54 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20170525035442.51407-1-dsahern@gmail.com>
Noticed these doing the extack support:
- nla_get_via is only used in af_mpls.c so remove from internal.h
- err is initialized to EINVAL in mpls_nh_build_from_cfg but then
set again before it is checked. Remove the EINVAL setting
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/mpls/af_mpls.c | 5 +++--
net/mpls/internal.h | 2 --
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index d0120f8c8a2c..ee468feb9e64 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -43,6 +43,9 @@ static void rtmsg_lfib(int event, u32 label, struct mpls_route *rt,
struct nlmsghdr *nlh, struct net *net, u32 portid,
unsigned int nlm_flags);
+static int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
+ u8 via[], struct netlink_ext_ack *extack);
+
static struct mpls_route *mpls_route_input_rcu(struct net *net, unsigned index)
{
struct mpls_route *rt = NULL;
@@ -695,8 +698,6 @@ static int mpls_nh_build_from_cfg(struct mpls_route_config *cfg,
if (!nh)
return -ENOMEM;
- err = -EINVAL;
-
nh->nh_labels = cfg->rc_output_labels;
for (i = 0; i < nh->nh_labels; i++)
nh->nh_label[i] = cfg->rc_output_label[i];
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index a015a6a1143b..cf65aec2e551 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -204,8 +204,6 @@ int nla_put_labels(struct sk_buff *skb, int attrtype, u8 labels,
const u32 label[]);
int nla_get_labels(const struct nlattr *nla, u8 max_labels, u8 *labels,
u32 label[], struct netlink_ext_ack *extack);
-int nla_get_via(const struct nlattr *nla, u8 *via_alen, u8 *via_table,
- u8 via[], struct netlink_ext_ack *extack);
bool mpls_output_possible(const struct net_device *dev);
unsigned int mpls_dev_mtu(const struct net_device *dev);
bool mpls_pkt_too_big(const struct sk_buff *skb, unsigned int mtu);
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* Re: [PATCH net-next] ibmvnic: Enable TSO support
From: kbuild test robot @ 2017-05-25 3:57 UTC (permalink / raw)
To: Thomas Falcon
Cc: kbuild-all, netdev, Thomas Falcon, Nathan Fontenot, John Allen
In-Reply-To: <1495679366-7149-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 2844 bytes --]
Hi Thomas,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Thomas-Falcon/ibmvnic-Enable-TSO-support/20170525-111039
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All warnings (new ones prefixed by >>):
In file included from include/linux/module.h:18:0,
from drivers/net/ethernet/ibm/ibmvnic.c:46:
drivers/net/ethernet/ibm/ibmvnic.c: In function '__check_large_send_offload':
include/linux/moduleparam.h:148:27: error: return from incompatible pointer type [-Werror=incompatible-pointer-types]
param_check_##type(name, &(value)); \
^
include/linux/moduleparam.h:346:68: note: in definition of macro '__param_check'
static inline type __always_unused *__check_##name(void) { return(p); }
^
include/linux/moduleparam.h:148:2: note: in expansion of macro 'param_check_bool'
param_check_##type(name, &(value)); \
^~~~~~~~~~~~
include/linux/moduleparam.h:128:2: note: in expansion of macro 'module_param_named'
module_param_named(name, name, type, perm)
^~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/ibm/ibmvnic.c:85:1: note: in expansion of macro 'module_param'
module_param(large_send_offload, bool, 0644);
^~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/module_param +85 drivers/net/ethernet/ibm/ibmvnic.c
69 #include <net/net_namespace.h>
70 #include <asm/hvcall.h>
71 #include <linux/atomic.h>
72 #include <asm/vio.h>
73 #include <asm/iommu.h>
74 #include <linux/uaccess.h>
75 #include <asm/firmware.h>
76 #include <linux/workqueue.h>
77 #include <linux/if_vlan.h>
78
79 #include "ibmvnic.h"
80
81 static const char ibmvnic_driver_name[] = "ibmvnic";
82 static const char ibmvnic_driver_string[] = "IBM System i/p Virtual NIC Driver";
83
84 static int large_send_offload;
> 85 module_param(large_send_offload, bool, 0644);
86 MODULE_PARM_DESC(large_send_offload,
87 "Determines whether large send offload is enabled");
88
89 MODULE_AUTHOR("Santiago Leon <santi_leon@yahoo.com>");
90 MODULE_DESCRIPTION("IBM System i/p Virtual NIC Driver");
91 MODULE_LICENSE("GPL");
92 MODULE_VERSION(IBMVNIC_DRIVER_VERSION);
93
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53754 bytes --]
^ permalink raw reply
* RE: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Ilan Tayari @ 2017-05-25 5:20 UTC (permalink / raw)
To: Alexei Starovoitov, Saeed Mahameed
Cc: David S. Miller, Doug Ledford,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jsorensen-b10kYP2dOMg@public.gmane.org
In-Reply-To: <20170524165059.42lmrwzzcxvi2sf7@ast-mbp>
> -----Original Message-----
> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
>
> On Tue, May 23, 2017 at 02:44:02PM +0300, Saeed Mahameed wrote:
> > From: Ilan Tayari <ilant@mellanox.com>
> >
> > Mellanox Innova is a NIC with ConnectX and an FPGA on the same
> > board. The FPGA is a bump-on-the-wire and thus affects operation of
> > the mlx5_core driver on the ConnectX ASIC.
> >
> > Add basic support for Innova in mlx5_core.
> >
> > This allows using the Innova card as a regular NIC, by detecting
> > the FPGA capability bit, and verifying its load state before
> > initializing ConnectX interfaces.
>
> That doesn't sound like cx4/cx5 hw that mlx5 driver suppose to support.
Hi Alexei,
Thanks for your feedback.
Let me address your concerns.
I didn't state it, but the current iterations of Innova cards include the
ConnectX-4LX chip, which is exactly what mlx5 driver supports.
The PCI interface is *only* through the ConnectX chip with some new
Firmware commands. This patch doesn't register any new PCI device,
so it does not make sense to create a separate driver.
>
> > Also detect FPGA fatal runtime failures and enter error state if
> > they ever happen.
> >
> > All new FPGA-related logic is placed in its own subdirectory 'fpga',
> > which may be built by selecting CONFIG_MLX5_FPGA.
> > This prepares for further support of various Innova features in later
> > patchsets.
> > Additional details about hardware architecture will be provided as
> > more features get submitted.
> >
> > Signed-off-by: Ilan Tayari <ilant@mellanox.com>
> > Reviewed-by: Boris Pismenny <borisp@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> > MAINTAINERS | 10 +
> > drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 10 +
> > drivers/net/ethernet/mellanox/mlx5/core/Makefile | 3 +
> > drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 ++
> > drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.c | 64 +++++++
> > drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.h | 59 ++++++
> > .../net/ethernet/mellanox/mlx5/core/fpga/core.c | 202
> +++++++++++++++++++++
> > .../net/ethernet/mellanox/mlx5/core/fpga/core.h | 99 ++++++++++
> > drivers/net/ethernet/mellanox/mlx5/core/main.c | 19 +-
> > include/linux/mlx5/device.h | 6 +
> > include/linux/mlx5/driver.h | 5 +
> > include/linux/mlx5/mlx5_ifc.h | 11 +-
> > include/linux/mlx5/mlx5_ifc_fpga.h | 144
> +++++++++++++++
> > 13 files changed, 640 insertions(+), 3 deletions(-)
>
> Can you put it into different driver? Dumping everything into by far
> the biggest nic driver already is already huge headache in terms on
> maintainability, debugging and back ports.
> Look at how intel splits their drivers.
> ixgb, ixgbe, ixgbevf are different drivers thought they have a lot in
> common. On one side it's a bit of copy paste, but on the other side
> it makes drivers much easier to develop and maintain independently.
> ConnectX-6 code and any future hw support doesn't belong to
> mlx5 driver at all.
If you build your driver without explicitly enabling CONFIG_MLX5_FPGA=y
(default is N) then you get none of this, and your driver is practically
the same as before.
The FPGA and CX operation is very tightly integrated.
If you don't want any of this, simply don't opt-in to CONFIG_MLX5_FPGA.
If you do want this, then splitting some of the logic to a
separate kernel object will not gain anything useful (logic would stay
the same), and just pollute the exported symbol table and open up the door
for issues of inter-module compatibility and so on.
Furthermore, the next patchset will introduce IPSec offload capabilities
Which are declared in netdev->hw_features. Those cannot be modified
after the netdevice is created, so all the extra logic has to be
integrated into the mlx5 ethernet driver. This is another reason why
a separate driver is a bad idea.
We will include details about the board and how things work in the
cover letter of the IPSec offload patchset.
Ilan.
^ permalink raw reply
* Re: [PATCH] bonding: Don't update slave->link until ready to commit
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-05-25 5:38 UTC (permalink / raw)
To: Nithin Nayak Sujir; +Cc: David Miller, linux-netdev, Jay Vosburgh
In-Reply-To: <1495680317-11308-1-git-send-email-nsujir@tintri.com>
On Wed, May 24, 2017 at 7:45 PM, Nithin Nayak Sujir <nsujir@tintri.com> wrote:
> In the loadbalance arp monitoring scheme, when a slave link change is
> detected, the slave->link is immediately updated and slave_state_changed
> is set. Later down the function, the rtnl_lock is acquired and the
> changes are committed, updating the bond link state.
>
> However, the acquisition of the rtnl_lock can fail. The next time the
> monitor runs, since slave->link is already updated, it determines that
> link is unchanged. This results in the bond link state permanently out
> of sync with the slave link.
>
> This patch modifies bond_loadbalance_arp_mon() to handle link changes
> identical to bond_ab_arp_{inspect/commit}(). The new link state is
> maintained in slave->new_link until we're ready to commit at which point
> it's copied into slave->link.
>
> NOTE: miimon_{inspect/commit}() has a more complex state machine
> requiring the use of the bond_{propose,commit}_link_state() functions
> which maintains the intermediate state in slave->link_new_state. The arp
> monitors don't require that.
>
> Testing: This bug is very easy to reproduce with the following steps.
> 1. In a loop, toggle a slave link of a bond slave interface.
> 2. In a separate loop, do ifconfig up/down of an unrelated interface to
> create contention for rtnl_lock.
> Within a few iterations, the bond link goes out of sync with the slave
> link.
>
> Signed-off-by: Nithin Nayak Sujir <nsujir@tintri.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
> Cc: Mahesh Bandewar <maheshb@google.com>
> Cc: Jay Vosburgh <jay.vosburgh@canonical.com>
> ---
> drivers/net/bonding/bond_main.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7331331..2359478b 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2612,11 +2612,13 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> bond_for_each_slave_rcu(bond, slave, iter) {
> unsigned long trans_start = dev_trans_start(slave->dev);
>
> + slave->new_link = BOND_LINK_NOCHANGE;
> +
> if (slave->link != BOND_LINK_UP) {
> if (bond_time_in_interval(bond, trans_start, 1) &&
> bond_time_in_interval(bond, slave->last_rx, 1)) {
>
> - slave->link = BOND_LINK_UP;
> + slave->new_link = BOND_LINK_UP;
> slave_state_changed = 1;
>
> /* primary_slave has no meaning in round-robin
> @@ -2643,7 +2645,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> if (!bond_time_in_interval(bond, trans_start, 2) ||
> !bond_time_in_interval(bond, slave->last_rx, 2)) {
>
> - slave->link = BOND_LINK_DOWN;
> + slave->new_link = BOND_LINK_DOWN;
> slave_state_changed = 1;
>
> if (slave->link_failure_count < UINT_MAX)
> @@ -2674,6 +2676,11 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> if (!rtnl_trylock())
> goto re_arm;
>
> + bond_for_each_slave(bond, slave, iter) {
> + if (slave->new_link != BOND_LINK_NOCHANGE)
> + slave->link = slave->new_link;
> + }
> +
> if (slave_state_changed) {
> bond_slave_state_change(bond);
> if (BOND_MODE(bond) == BOND_MODE_XOR)
> --
> 2.8.2
>
^ permalink raw reply
* [PATCH] net: llc: add lock_sock in llc_ui_bind to avoid a race condition
From: Lin Zhang @ 2017-05-25 6:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Lin Zhang
There is a race condition in llc_ui_bind if two or more processes/threads
try to bind a same socket.
If more processes/threads bind a same socket success that will lead to
two problems, one is this action is not what we expected, another is
will lead to kernel in unstable status or oops(in my simple test case,
cause llc2.ko can't unload).
The current code is test SOCK_ZAPPED bit to avoid a process to
bind a same socket twice but that is can't avoid more processes/threads
try to bind a same socket at the same time.
So, add lock_sock in llc_ui_bind like others, such as llc_ui_connect.
Signed-off-by: Lin Zhang <xiaolou4617@gmail.com>
---
net/llc/af_llc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 8364fe5..c38d16f 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -311,6 +311,8 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
int rc = -EINVAL;
dprintk("%s: binding %02X\n", __func__, addr->sllc_sap);
+
+ lock_sock(sk);
if (unlikely(!sock_flag(sk, SOCK_ZAPPED) || addrlen != sizeof(*addr)))
goto out;
rc = -EAFNOSUPPORT;
@@ -382,6 +384,7 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
out_put:
llc_sap_put(sap);
out:
+ release_sock(sk);
return rc;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net-next v2 0/8] mlxsw: Support firmware flash
From: Jiri Pirko @ 2017-05-25 6:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev, idosch, yotamg, mlxsw, Yuval.Mintz
In-Reply-To: <20170524.163328.672261790569871029.davem@davemloft.net>
Wed, May 24, 2017 at 10:33:28PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Tue, 23 May 2017 21:56:22 +0200
>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Add support for device firmware flash on mlxsw spectrum. The firmware files
>> are expected to be in the Mellanox Firmware Archive version 2 (MFA2)
>> format.
>>
>> The firmware flash is triggered on driver initialization time if the device
>> firmware version does not meet the minimum firmware version supported by
>> the driver.
>>
>> Currently, to activate the newly flashed firmware, the user needs to
>> reboot his system.
>>
>> The first patch introduces the mlxfw module, which implements common logic
>> needed for the firmware flash process on Mellanox products, such as the
>> MFA2 format parsing and the firmware flash state machine logic. As the
>> module implements common logic which will be needed by various different
>> Mellanox drivers, it defines a set of callbacks needed to interact with the
>> specific device.
>>
>> Patches 1-5 implement the needed mlxfw callbacks in the mlxsw spectrum
>> driver.
>>
>> Patches 6 and 7 add boot-time firmware upgrade on the mlxsw spectrum
>> driver.
>>
>> Patch 8 adds a fix needed for new firmware versions.
>
>Series applied, although I hope you sort out the user interface
>soon otherwise most of this is completely dead unused code.
No dead code. The flashing is used internally from the driver on the
modprobe time. We are looking into the user interface at the moment.
Thanks.
>
>Thanks.
^ permalink raw reply
* Re: [PATCH net-next 2/4] nfp: register ports as devlink ports
From: Jiri Pirko @ 2017-05-25 8:20 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20170524122413.702e0eef@cakuba.netronome.com>
Wed, May 24, 2017 at 09:24:13PM CEST, kubakici@wp.pl wrote:
>On Wed, 24 May 2017 14:35:14 +0200, Jiri Pirko wrote:
>> >+void nfp_devlink_port_unregister(struct nfp_port *port)
>> >+{
>> >+ /* Due to unpleasant lock ordering we may see the port go away before
>> >+ * we have fully probed.
>>
>> Could you elaborate on this a bit more please?
>
>It's partially due to peculiarities of the management FW more than
>kernel stuff. Unfortunately some ethtool media config requires reboot
>to be applied, so we print a friendly message to the logs and
>unregister the associated netdevs. Which means once netdevs get
>registered ports may go away.
>
>Enter devlink, I need the ability to grab the adapater lock in
>split/unsplit callbacks to find the ports, which implies having to drop
>that lock before I register devlink. And only after I register devlink
>can I register the ports.
>
>I could do init without registering anything, drop the adapter lock,
>register devlink, and then grab the adapter lock back and register
>devlink ports and netdevs. But there is another issue...
>
>Since I look for ports on a list maintained in the adapter struct,
>driver code doesn't care if devlink_port has been registered or not.
>The moment devlink is registered, split/unsplit requests will be
>accepted - potentially trying to unregister devlink_port before the
>register could happen.
>
>Further down the line, also, the eswitch mode setting is coming. Which
>means the moment I register devlink itself ports will get shuffled (due
>to the plan of registering VFs as ports :)).
>
>I feel like registering devlink should be the last action of the
>driver, really. My plan was to keep that simple if() for now, and once
>we get to extending devlink with SR-IOV stuff also add the ability to
>pre-register ports. Allow registering ports on not-yet-registered
>devlink (probably put them on a private list within struct devlink).
>This would make devlink_register() a single point when everything
>devlink becomes visible, atomically, instead of devlink itself coming
>first and then ports following.
>
>Does that make sense? Am I misreading the code (again :S)?
Well in mlxsw, we internally maintain a list of port and we assign a
pointer to the port struct only after all is initialized:
...
mlxsw_sp->ports[local_port] = mlxsw_sp_port;
err = register_netdev(dev);
...
Then in split, we check:
mlxsw_sp_port = mlxsw_sp->ports[local_port];
if (!mlxsw_sp_port) {
dev_err(mlxsw_sp->bus_info->dev, "Port number \"%d\" does not exist\n",
local_port);
return -EINVAL;
}
I guess that you can do something similar. You should ensure that split
won't happen for half-initialized ports.
^ permalink raw reply
* Re: [PATCH] i40e: Fix incorrect pf->flags
From: Jeff Kirsher @ 2017-05-25 9:13 UTC (permalink / raw)
To: Tushar Dave, intel-wired-lan, netdev
In-Reply-To: <1495242112-14835-1-git-send-email-tushar.n.dave@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]
On Fri, 2017-05-19 at 18:01 -0700, Tushar Dave wrote:
> Fix bug introduced by 'commit 47994c119a36e ("i40e: remove
> hw_disabled_flags in favor of using separate flag bits")' that
> mistakenly wipes out pf->flags.
>
> Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
:-(
Not even close to applying, I even tried to apply by hand but the patch
was not near the same as my branch in my tree. I will forgive this
second instance of a patch generated on different tree/branch, but I
really must insist that you use my tree & branch if you need/want to
make changes to Intel wired LAN driver changes.
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index d5c9c9e..6b98d34 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -8821,9 +8821,9 @@ static int i40e_sw_init(struct i40e_pf *pf)
> (pf->hw.aq.api_min_ver > 4))) {
> /* Supported in FW API version higher than 1.4 */
> pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
> - pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> + pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> } else {
> - pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> + pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
> }
>
> pf->eeprom_version = 0xDEAD;
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Saeed Mahameed @ 2017-05-25 10:40 UTC (permalink / raw)
To: Ilan Tayari
Cc: Alexei Starovoitov, Saeed Mahameed, David S. Miller, Doug Ledford,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jsorensen-b10kYP2dOMg@public.gmane.org
In-Reply-To: <AM4PR0501MB1940885C2F1CEF4DDE4EA8D1DBFF0-dp/nxUn679gfNUYDR5dMTsDSnupUy6xnnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On Thu, May 25, 2017 at 8:20 AM, Ilan Tayari <ilant-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> -----Original Message-----
>> From: Alexei Starovoitov [mailto:alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>>
>> On Tue, May 23, 2017 at 02:44:02PM +0300, Saeed Mahameed wrote:
>> > From: Ilan Tayari <ilant-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >
>> > Mellanox Innova is a NIC with ConnectX and an FPGA on the same
>> > board. The FPGA is a bump-on-the-wire and thus affects operation of
>> > the mlx5_core driver on the ConnectX ASIC.
>> >
>> > Add basic support for Innova in mlx5_core.
>> >
>> > This allows using the Innova card as a regular NIC, by detecting
>> > the FPGA capability bit, and verifying its load state before
>> > initializing ConnectX interfaces.
>>
>> That doesn't sound like cx4/cx5 hw that mlx5 driver suppose to support.
>
> Hi Alexei,
> Thanks for your feedback.
> Let me address your concerns.
>
> I didn't state it, but the current iterations of Innova cards include the
> ConnectX-4LX chip, which is exactly what mlx5 driver supports.
>
> The PCI interface is *only* through the ConnectX chip with some new
> Firmware commands. This patch doesn't register any new PCI device,
> so it does not make sense to create a separate driver.
>
>>
>> > Also detect FPGA fatal runtime failures and enter error state if
>> > they ever happen.
>> >
>> > All new FPGA-related logic is placed in its own subdirectory 'fpga',
>> > which may be built by selecting CONFIG_MLX5_FPGA.
>> > This prepares for further support of various Innova features in later
>> > patchsets.
>> > Additional details about hardware architecture will be provided as
>> > more features get submitted.
>> >
>> > Signed-off-by: Ilan Tayari <ilant-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > Reviewed-by: Boris Pismenny <borisp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > Signed-off-by: Saeed Mahameed <saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> > ---
>> > MAINTAINERS | 10 +
>> > drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 10 +
>> > drivers/net/ethernet/mellanox/mlx5/core/Makefile | 3 +
>> > drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 ++
>> > drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.c | 64 +++++++
>> > drivers/net/ethernet/mellanox/mlx5/core/fpga/cmd.h | 59 ++++++
>> > .../net/ethernet/mellanox/mlx5/core/fpga/core.c | 202
>> +++++++++++++++++++++
>> > .../net/ethernet/mellanox/mlx5/core/fpga/core.h | 99 ++++++++++
>> > drivers/net/ethernet/mellanox/mlx5/core/main.c | 19 +-
>> > include/linux/mlx5/device.h | 6 +
>> > include/linux/mlx5/driver.h | 5 +
>> > include/linux/mlx5/mlx5_ifc.h | 11 +-
>> > include/linux/mlx5/mlx5_ifc_fpga.h | 144
>> +++++++++++++++
>> > 13 files changed, 640 insertions(+), 3 deletions(-)
>>
>> Can you put it into different driver? Dumping everything into by far
>> the biggest nic driver already is already huge headache in terms on
>> maintainability, debugging and back ports.
>> Look at how intel splits their drivers.
>> ixgb, ixgbe, ixgbevf are different drivers thought they have a lot in
>> common. On one side it's a bit of copy paste, but on the other side
I don't think the ixgb example is the same, simply ixgb, ixgbe,
ixgbevf have different PCI IDs
and even different SW/FW interfaces. On the other hand, same mlx5
driver can support all of
ConnetX4/5/6 device IDs with the same code flows, same interfaces.
>> it makes drivers much easier to develop and maintain independently.
>> ConnectX-6 code and any future hw support doesn't belong to
>> mlx5 driver at all.
Sorry i must disagree with you on this for the same reasons Ilan mentioned.
We can perfectly achieve the same with modular driver design all under the
same .ko module, with some kconfig flags to reduce the amount of code/features
this .ko provides.
>
> If you build your driver without explicitly enabling CONFIG_MLX5_FPGA=y
> (default is N) then you get none of this, and your driver is practically
> the same as before.
>
> The FPGA and CX operation is very tightly integrated.
> If you don't want any of this, simply don't opt-in to CONFIG_MLX5_FPGA.
>
> If you do want this, then splitting some of the logic to a
> separate kernel object will not gain anything useful (logic would stay
> the same), and just pollute the exported symbol table and open up the door
> for issues of inter-module compatibility and so on.
>
> Furthermore, the next patchset will introduce IPSec offload capabilities
> Which are declared in netdev->hw_features. Those cannot be modified
> after the netdevice is created, so all the extra logic has to be
> integrated into the mlx5 ethernet driver. This is another reason why
> a separate driver is a bad idea.
>
> We will include details about the board and how things work in the
> cover letter of the IPSec offload patchset.
>
> Ilan.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 v9 06/15] mlx5: Replace PCI pool old API
From: Romain Perier @ 2017-05-25 11:37 UTC (permalink / raw)
To: David Miller
Cc: leon, dan.j.williams, dledford, sean.hefty, hal.rosenstock,
jeffrey.t.kirsher, stas.yakovlev, jejb, martin.petersen, balbi,
gregkh, linux-rdma, netdev, linux-usb, linux-scsi, linux-kernel,
peter.senna
In-Reply-To: <20170523.112658.1479025487863881513.davem@davemloft.net>
Hello,
Le 23/05/2017 à 17:26, David Miller a écrit :
> From: Romain Perier <romain.perier@collabora.com>
> Date: Tue, 23 May 2017 10:53:36 +0200
>
>> Hello,
>>
>>
>> Le 23/05/2017 à 09:27, Leon Romanovsky a écrit :
>>> On Mon, May 22, 2017 at 06:48:58PM +0200, Romain Perier wrote:
>>>> The PCI pool API is deprecated. This commit replaces the PCI pool old
>>>> API by the appropriate function with the DMA pool API.
>>>>
>>>> Signed-off-by: Romain Perier <romain.perier@collabora.com>
>>>> Reviewed-by: Peter Senna Tschudin <peter.senna@collabora.com>
>>>> Acked-by: Doug Ledford <dledford@redhat.com>
>>>> Tested-by: Doug Ledford <dledford@redhat.com>
>>>> ---
>>>> drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 11 ++++++-----
>>>> include/linux/mlx5/driver.h | 2 +-
>>>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>>>
>>> Who is supposed to merge this patch series?
>>>
>>> Acked-by: Leon Romanovsky <leonro@mellanox.com>
>> Each maintainer of the corresponding subsystem, can take a patch, I
>> guess. No ?
> It might be easier to accumulate acks and you submit them as a series,
> in my opinion.
That's what I did already, no ?
Romain
^ permalink raw reply
* RE: [PATCH] dt-bindings: net: move FMan binding
From: Madalin-Cristian Bucur @ 2017-05-25 12:57 UTC (permalink / raw)
To: David Miller, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org,
oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org,
paulus-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org,
mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170515.103048.248884533404807350.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> -----Original Message-----
> From: David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
> Sent: Monday, May 15, 2017 5:31 PM
> Subject: Re: [PATCH] dt-bindings: net: move FMan binding
>
> From: Madalin Bucur <madalin.bucur-3arQi8VN3Tc@public.gmane.org>
> Date: Mon, 15 May 2017 16:36:38 +0300
>
> > Besides the PPC SoCs, the QorIQ DPAA FMan is also present on ARM SoCs,
> > moving the device tree binding document into the bindings/net folder.
> >
> > Signed-off-by: Madalin Bucur <madalin.bucur-3arQi8VN3Tc@public.gmane.org>
>
> What tree is this targetted at for merging?
I hope Rob Herring will take this into his tree, it's a device tree binding
change.
Thanks,
Madalin
--
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 net] net/mlx4: Fix the check in attaching steering rules
From: Tariq Toukan @ 2017-05-25 13:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eranbe, ogerlitz, talatb
In-Reply-To: <20170524.153625.1291847374941790972.davem@davemloft.net>
On 24/05/2017 10:36 PM, David Miller wrote:
> From: Tariq Toukan <tariqt@mellanox.com>
> Date: Tue, 23 May 2017 15:50:07 +0300
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>> index ae5fdc2df654..00a7cd3dcc2e 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>> @@ -1562,8 +1562,7 @@ static int mlx4_en_flow_replace(struct net_device *dev,
>> qpn = priv->drop_qp.qpn;
>> else if (cmd->fs.ring_cookie & EN_ETHTOOL_QP_ATTACH) {
>> qpn = cmd->fs.ring_cookie & (EN_ETHTOOL_QP_ATTACH - 1);
>> - if (qpn < priv->rss_map.base_qpn ||
>> - qpn >= priv->rss_map.base_qpn + priv->rx_ring_num) {
>> + if (!mlx4_qp_lookup(priv->mdev->dev, qpn)) {
>> en_warn(priv, "rxnfc: QP (0x%x) doesn't exist\n", qpn);
>> return -EINVAL;
>> }
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c b/drivers/net/ethernet/mellanox/mlx4/qp.c
>> index 2d6abd4662b1..1eff2fe32a8b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/qp.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
>> @@ -384,6 +384,20 @@ static void mlx4_qp_free_icm(struct mlx4_dev *dev, int qpn)
>> __mlx4_qp_free_icm(dev, qpn);
>> }
>>
>> +struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn)
> ...
>> +EXPORT_SYMBOL_GPL(mlx4_qp_lookup);
>> +
>
> This phony separation between MLX4_CORE and MLX4_EN is the only reason
> you need this unreasonable symbol export.
>
> I doubt you'll ever use this function anywhere outside of en_ethtool.c
> so this export is wasted space in the kernel image. Probably compiler
> could inline it decently as well.
>
> So find another way to do this without the symbol export. I don't
> really want to hear any stories about "clean separation" or whatever.
> What's happening here is exactly why this separate modules scheme
> results in ugly unreasonable code, and unnecessary gymnastics and
> wasted object space just to make routines available in one place from
> another.
I see. I'll remove this EXPORT_SYMBOL_GPL and send a re-spin shortly.
Function will still be accessible within EN driver, as mlx4_en.h includes qp.h.
Regards,
Tariq
^ 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