* [PATCH] net: rose: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-19 18:03 UTC (permalink / raw)
To: Ralf Baechle, David S. Miller
Cc: linux-hams, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/rose/rose_in.c | 1 +
net/rose/rose_route.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/net/rose/rose_in.c b/net/rose/rose_in.c
index 0a63947..9bbbfe3 100644
--- a/net/rose/rose_in.c
+++ b/net/rose/rose_in.c
@@ -219,6 +219,7 @@ static int rose_state4_machine(struct sock *sk, struct sk_buff *skb, int framety
switch (frametype) {
case ROSE_RESET_REQUEST:
rose_write_internal(sk, ROSE_RESET_CONFIRMATION);
+ /* fall through */
case ROSE_RESET_CONFIRMATION:
rose_stop_timer(sk);
rose_start_idletimer(sk);
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 452bbb3..ac0f733 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -346,6 +346,7 @@ static int rose_del_node(struct rose_route_struct *rose_route,
case 0:
rose_node->neighbour[0] =
rose_node->neighbour[1];
+ /* fall through */
case 1:
rose_node->neighbour[1] =
rose_node->neighbour[2];
@@ -507,6 +508,7 @@ void rose_rt_device_down(struct net_device *dev)
switch (i) {
case 0:
t->neighbour[0] = t->neighbour[1];
+ /* fall through */
case 1:
t->neighbour[1] = t->neighbour[2];
case 2:
--
2.7.4
^ permalink raw reply related
* RE: [PATCH 2/7] devlink: Adding NPAR permanent config parameters
From: Yuval Mintz @ 2017-10-19 18:08 UTC (permalink / raw)
To: Steve Lin
Cc: netdev@vger.kernel.org, Jiri Pirko, davem@davemloft.net,
michael.chan@broadcom.com, linville@tuxdriver.com,
gospo@broadcom.com
In-Reply-To: <CA+Jmh7FEcXij7AEQmWmU8mz_=DW-SYpoyKiSESPW7aeuJuV4fw@mail.gmail.com>
> >> DEVLINK_ATTR_PERM_CFG_NPAR_BW_RESERVATION_VALID: 1 to use
> >> BW_RESERVATION setting, 0 to ignore.
> >>
> > ...
> >> DEVLINK_ATTR_PERM_CFG_NPAR_BW_LIMIT_VALID: 1 to use BW_LIMIT
> >> setting, 0 to ignore.
> >
> > While it probably ties to different fields in your NVM layout why would the
> user
> > require specific attributes for these? Why not have values in the actual
> > attributes indicating of this status?
>
> Hi Yuval,
>
> Does having the separate valid flag present any difficulties? There
> are lots of implementation options here (a limit or reservation value
> of 0 could mean invalid, or we could define (1 << 31) to be a valid
> flag when setting the value, etc.), and I'm not necessarily tied to
> doing it this way, but it seemed a straightforward way to represent
> the validity of the other field.
You're pushing a LOT of new attributes, every one of which is going
to have to be documented for future generations.
I think whenever it's possible to drop an unnecessary attribute, that
would be the better option.
^ permalink raw reply
* Kernel 3.0.35 RT - Kernel OOPS error on enet interrupt
From: Sebastien Bourdelin @ 2017-10-19 18:33 UTC (permalink / raw)
To: netdev; +Cc: Jerome Oufella, Ash Charles
Hi,
I'm currently stuck with a Kernel OOPS errors which seems to happen when
my system is under heavy load.
The steps to reproduce the problem are unknown from me.
The bug happens when accessing to the __wake_up_common at the following
line:
http://elixir.free-electrons.com/linux/v3.0.35/source/kernel/sched.c#L4456
It seems i'm trying to access a task in a workqueue which has been
poisoning.
I'm wondering if the following calltrace triggers something to you or
what could cause this behavior.
Best Regards,
Sebastien.
Unable to handle kernel paging request at virtual address 00100100
pgd = 80004000
[00100100] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT SMP
Modules linked in: sd8xxx mlan(P)
CPU: 3 Tainted: P (3.0.35-rt56-PD13.2.4-Oct-B421 #1)
PC is at __wake_up_common+0x78/0x90
LR is at try_to_wake_up+0x5c/0x2f4
pc : [<800c5c64>] lr : [<800cf5f0>] psr: 60000013
sp : ba2ade18 ip : 001000f4 fp : ba2ade44
r10: 00000001 r9 : 000000c3 r8 : 00000001
r7 : ba457e3c r6 : 00000000 r5 : 80166620 r4 : 001000f4
r3 : 001000f4 r2 : 00000000 r1 : 20000013 r0 : 00000000
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 4a5b404a DAC: 00000015
Process irq/150-enet (pid: 1156, stack limit = 0xba2ac2f0)
Stack: (0xba2ade18 to 0xba2ae000)
de00: 8c0272e0
00000001
de20: 8c0272e0 ba457e20 000000c3 00000001 00000001 00002000 bfd16000
0000004a
de40: ba2ade64 800ce9b0 000000c3 ba809520 ba5d6c00 ba5d6c80 ba5d6c00
ba77c02e
de60: 00000058 8040bf68 ba809520 804e0d8c bfd16000 8072a744 8072a764
ba2adeac
de80: 00000152 bfd16000 00000000 00000000 8072a208 80417980 bfd16000
80418b94
dea0: 8077ea60 ba809520 8c026320 8072a764 807161c0 ba809520 ba2adeec
ffdf53e0
dec0: 00000800 bfd1706c 00000058 bfd1706c 00000800 8041b49c 4a77c810
800ae6b0
dee0: ba77c020 000007f0 00000002 ba2adee4 bfd16000 bfd16000 ffdf53e0
8032b74c
df00: 00000096 00000002 ba809520 8074db9c ba702240 02000000 ffdf53e0
bfd16bcc
df20: 00000000 bfd16c58 4a61a020 bfd1706c bfd17488 bfd17490 80716220
8071620c
df40: 00000000 ba2ac000 ba2ac000 807161c0 807161c0 bfc18220 00000001
bfc18220
df60: 8072a034 80716220 8071620c 8010f850 ba2ac000 807161c0 bfc1823c
8010f768
df80: bffdfe3c 8010f830 bfc18220 00000008 00000013 bffdfe2c bfc18220
8010f620
dfa0: 00000013 00000000 00000000 00000000 00000000 800edf90 00000000
bfc18220
dfc0: 00000000 00000000 00000000 ba2adfcc ba2adfcc 00000000 00000000
00000000
dfe0: ba2adfe0 ba2adfe0 bffdfe2c 800edf10 800a1368 800a1368 a79596d4
c2e48e91
[<800c5c64>] (__wake_up_common+0x78/0x90) from [<800ce9b0>]
(__wake_up_sync_key+0x48/0x64)
[<800ce9b0>] (__wake_up_sync_key+0x48/0x64) from [<8040bf68>]
(sock_def_readable+0x3c/0x64)
[<8040bf68>] (sock_def_readable+0x3c/0x64) from [<804e0d8c>]
(packet_rcv+0x2d8/0x3b8)
[<804e0d8c>] (packet_rcv+0x2d8/0x3b8) from [<80417980>]
(__netif_receive_skb+0x1ac/0x390)
[<80417980>] (__netif_receive_skb+0x1ac/0x390) from [<8041b49c>]
(netif_receive_skb+0xa4/0xb4)
[<8041b49c>] (netif_receive_skb+0xa4/0xb4) from [<8032b74c>]
(fec_enet_interrupt+0x4f8/0x8a0)
[<8032b74c>] (fec_enet_interrupt+0x4f8/0x8a0) from [<8010f850>]
(irq_forced_thread_fn+0x20/0x58)
[<8010f850>] (irq_forced_thread_fn+0x20/0x58) from [<8010f768>]
(irq_thread+0x148/0x1d8)
[<8010f768>] (irq_thread+0x148/0x1d8) from [<800edf90>] (kthread+0x80/0x88)
[<800edf90>] (kthread+0x80/0x88) from [<800a1368>]
(kernel_thread_exit+0x0/0x8)
Code: e51b2028 e2522001 e50b2028 0a000003 (e5b3400c)
^ permalink raw reply
* RE: [PATCH 4/7] devlink: Adding perm config of link settings
From: Yuval Mintz @ 2017-10-19 18:34 UTC (permalink / raw)
To: Steve Lin
Cc: netdev@vger.kernel.org, Jiri Pirko, davem@davemloft.net,
michael.chan@broadcom.com, linville@tuxdriver.com,
gospo@broadcom.com
In-Reply-To: <CA+Jmh7H+u1-1Ot2TxLXEHk2HW=p+yeT02cexNPC1nW5V_gMVgQ@mail.gmail.com>
> On Thu, Oct 19, 2017 at 2:07 AM, Yuval Mintz <yuvalm@mellanox.com>
> wrote:
> >> +enum devlink_autoneg_protocol {
> >> + DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_BAM,
> >> + DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY_CONSORTIUM,
> >> + DEVLINK_AUTONEG_PROTOCOL_IEEE8023BY,
> >> + DEVLINK_AUTONEG_PROTOCOL_BAM, /* Broadcom
> >> Autoneg Mode */
> >> + DEVLINK_AUTONEG_PROTOCOL_CONSORTIUM, /*
> >> Consortium Autoneg Mode */
> >> +};
> >
> > Wouldn't adding BAM as a 'generic' mode of operation be like adding
> > non-consortium speeds to ethtool API?
> > [I profess ignorance in this area; For all I know it can be a widely accepted
> > industry standard]
> >
>
> Yuval, I'm glad to get input from other NIC vendors.
Other switch vendors ;)
>The high-level goal of this effort is to allow users of various vendors' NICs to be
> able to configure these types of NVRAM/permanent/default settings
> using an inbox tool, rather than the collection of vendor-specific
> tools that is the status quo.
>
> In order to provide that functionality, it seems like the
> vendor-specific parameters and also the vendor-specific settings of
> common parameters both need to be supported in this manner.
>
> Ideally there will be much overlap in both the set of parameters
> available as well as the options for each parameter, but in the real
> world, there will always be differences between vendors and even
> between different devices (drivers) from the same vendor. Despite
> that reality, I think there is still great benefit in having a common
> inbox tool that users can use for device configuration of this type.
> It just means that not all drivers will support all parameters, nor
> all options for each parameter that they do support.
I don't object the end-goal; I think my hesitation is due to the same
enum containing both generic and vendor-specific values mixed
together. I feel like we need some clear distinction between the two.
>
> Thanks,
> Steve
^ permalink raw reply
* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Oliver Hartkopp @ 2017-10-19 18:35 UTC (permalink / raw)
To: Marc Kleine-Budde, Sekhar Nori, Franklin S Cooper Jr,
Mario Hüttel, Yang, Wenyou, wg, quentin.schulz, edumazet,
linux-can, netdev, linux-kernel
Cc: Wenyou Yang, Dong Aisheng, Quadros, Roger
In-Reply-To: <0099eefa-32c8-a818-761c-667239d9ec3b@pengutronix.de>
Hi Marc,
On 10/19/2017 01:26 PM, Marc Kleine-Budde wrote:
> On 10/19/2017 01:14 PM, Oliver Hartkopp wrote:
>>>>>> Since we have a netlink socket interface to configure sample point, I
>>>>>> wonder if that should be extended to configure SSP too (or at least the
>>>>>> offset part of SSP)?
>>
>> +1 too
>
> The struct can_bittiming in defined in uapi, so we have to keep ABI
> compatibility in mind.
>
Oh, this is fortunately NO problem ;-)
struct can_bittiming {
__u32 bitrate; /* Bit-rate in bits/second */
__u32 sample_point; /* Sample point in one-tenth of a
percent */
__u32 tq; /* Time quanta (TQ) in nanoseconds */
__u32 prop_seg; /* Propagation segment in TQs */
__u32 phase_seg1; /* Phase buffer segment 1 in TQs */
__u32 phase_seg2; /* Phase buffer segment 2 in TQs */
__u32 sjw; /* Synchronisation jump width in TQs */
__u32 brp; /* Bit-rate prescaler */
};
So we have two of these: One for the arbitration bitrate and one
sample_point for the data bitrate -> the 'secondary' SP -> SSP
:-)
We already have this 'dsample-point' implemented in the ip tool:
$ ip link set vcan0 type can help
Usage: ip link set DEVICE type can
[ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
[ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1
phase-seg2 PHASE-SEG2 [ sjw SJW ] ]
[ dbitrate BITRATE [ dsample-point SAMPLE-POINT] ] | <<-- here!
[ dtq TQ dprop-seg PROP_SEG dphase-seg1 PHASE-SEG1
dphase-seg2 PHASE-SEG2 [ dsjw SJW ] ]
But AFAIK m_can is not using that value in m_can_set_bittiming().
>>> If good default values are transceiver and board specific, they can go
>>> into the DT. We need a generic (this means driver agnostic) binding for
>>> this. If this table needs to be tweaked for special purpose, then we can
>>> add a netlink interface for this as well. >
>>> Comments?
>>
>> By now we calculate reasonable default values (e.g. for SP and SJW), you
>> can override by setting alternative values via netlink configuration.
>>
>> I would tend to stay on this approach and not hide these things in DTs -
>> just because of someone wants to initialize his specific interface 'easier'.
>
> If the values are not board specific, then it makes no sense to put them
> into the DT.
When they are NOT(?) board specific?
Thinking about non-SoC CAN adapters with PCI and USB pushing the SSP to
the DT looks wrong to me.
Best,
Oliver
^ permalink raw reply
* [PATCH] net: rxrpc: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-19 18:50 UTC (permalink / raw)
To: David Howells, David S. Miller
Cc: linux-afs, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/rxrpc/af_rxrpc.c | 5 +++--
net/rxrpc/input.c | 2 +-
net/rxrpc/sendmsg.c | 1 +
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index fb17552..df4f7d5 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -246,6 +246,7 @@ static int rxrpc_listen(struct socket *sock, int backlog)
ret = 0;
break;
}
+ /* fall through */
default:
ret = -EBUSY;
break;
@@ -528,8 +529,7 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len)
rx->local = local;
rx->sk.sk_state = RXRPC_CLIENT_UNBOUND;
- /* Fall through */
-
+ /* fall through */
case RXRPC_CLIENT_UNBOUND:
case RXRPC_CLIENT_BOUND:
if (!m->msg_name &&
@@ -537,6 +537,7 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len)
m->msg_name = &rx->connect_srx;
m->msg_namelen = sizeof(rx->connect_srx);
}
+ /* fall through */
case RXRPC_SERVER_BOUND:
case RXRPC_SERVER_LISTENING:
ret = rxrpc_do_sendmsg(rx, m, len);
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index e56e23e..8bc6daf 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -1125,7 +1125,7 @@ void rxrpc_data_ready(struct sock *udp_sk)
case RXRPC_PACKET_TYPE_BUSY:
if (sp->hdr.flags & RXRPC_CLIENT_INITIATED)
goto discard;
-
+ /* fall through */
case RXRPC_PACKET_TYPE_DATA:
if (sp->hdr.callNumber == 0)
goto bad_message;
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 9ea6f97..d245782 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -166,6 +166,7 @@ static void rxrpc_queue_packet(struct rxrpc_sock *rx, struct rxrpc_call *call,
ktime_get_real());
if (!last)
break;
+ /* fall through */
case RXRPC_CALL_SERVER_SEND_REPLY:
call->state = RXRPC_CALL_SERVER_AWAIT_ACK;
rxrpc_notify_end_tx(rx, call, notify_end_tx);
--
2.7.4
^ permalink raw reply related
* [PATCH net] soreuseport: fix initialization race
From: Craig Gallek @ 2017-10-19 19:00 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev
From: Craig Gallek <kraig@google.com>
Syzkaller stumbled upon a way to trigger
WARNING: CPU: 1 PID: 13881 at net/core/sock_reuseport.c:41
reuseport_alloc+0x306/0x3b0 net/core/sock_reuseport.c:39
There are two initialization paths for the sock_reuseport structure in a
socket: Through the udp/tcp bind paths of SO_REUSEPORT sockets or through
SO_ATTACH_REUSEPORT_[CE]BPF before bind. The existing implementation
assumedthat the socket lock protected both of these paths when it actually
only protects the SO_ATTACH_REUSEPORT path. Syzkaller triggered this
double allocation by running these paths concurrently.
This patch moves the check for double allocation into the reuseport_alloc
function which is protected by a global spin lock.
Fixes: e32ea7e74727 ("soreuseport: fast reuseport UDP socket selection")
Fixes: c125e80b8868 ("soreuseport: fast reuseport TCP socket selection")
Signed-off-by: Craig Gallek <kraig@google.com>
---
net/core/sock_reuseport.c | 12 +++++++++---
net/ipv4/inet_hashtables.c | 5 +----
net/ipv4/udp.c | 5 +----
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index eed1ebf7f29d..b1e0dbea1e8c 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -36,9 +36,14 @@ int reuseport_alloc(struct sock *sk)
* soft irq of receive path or setsockopt from process context
*/
spin_lock_bh(&reuseport_lock);
- WARN_ONCE(rcu_dereference_protected(sk->sk_reuseport_cb,
- lockdep_is_held(&reuseport_lock)),
- "multiple allocations for the same socket");
+
+ /* Allocation attempts can occur concurrently via the setsockopt path
+ * and the bind/hash path. Nothing to do when we lose the race.
+ */
+ if (rcu_dereference_protected(sk->sk_reuseport_cb,
+ lockdep_is_held(&reuseport_lock)))
+ goto out;
+
reuse = __reuseport_alloc(INIT_SOCKS);
if (!reuse) {
spin_unlock_bh(&reuseport_lock);
@@ -49,6 +54,7 @@ int reuseport_alloc(struct sock *sk)
reuse->num_socks = 1;
rcu_assign_pointer(sk->sk_reuseport_cb, reuse);
+out:
spin_unlock_bh(&reuseport_lock);
return 0;
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 597bb4cfe805..e7d15fb0d94d 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -456,10 +456,7 @@ static int inet_reuseport_add_sock(struct sock *sk,
return reuseport_add_sock(sk, sk2);
}
- /* Initial allocation may have already happened via setsockopt */
- if (!rcu_access_pointer(sk->sk_reuseport_cb))
- return reuseport_alloc(sk);
- return 0;
+ return reuseport_alloc(sk);
}
int __inet_hash(struct sock *sk, struct sock *osk)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index e45177ceb0ee..3cd8103bab2c 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -231,10 +231,7 @@ static int udp_reuseport_add_sock(struct sock *sk, struct udp_hslot *hslot)
}
}
- /* Initial allocation may have already happened via setsockopt */
- if (!rcu_access_pointer(sk->sk_reuseport_cb))
- return reuseport_alloc(sk);
- return 0;
+ return reuseport_alloc(sk);
}
/**
--
2.15.0.rc1.287.g2b38de12cc-goog
^ permalink raw reply related
* [PATCH] igb: Fix TX map failure path
From: Jean-Philippe Brucker @ 2017-10-19 19:07 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: intel-wired-lan, netdev
When the driver cannot map a TX buffer, instead of rolling back
gracefully and retrying later, we currently get a panic:
[ 159.885994] igb 0000:00:00.0: TX DMA map failed
[ 159.886588] Unable to handle kernel paging request at virtual address ffff00000a08c7a8
...
[ 159.897031] PC is at igb_xmit_frame_ring+0x9c8/0xcb8
Fix the erroneous test that leads to this situation.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index fd4a46b03cc8..ea69af267d63 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5326,7 +5326,7 @@ static int igb_tx_map(struct igb_ring *tx_ring,
DMA_TO_DEVICE);
dma_unmap_len_set(tx_buffer, len, 0);
- if (i--)
+ if (i-- == 0)
i += tx_ring->count;
tx_buffer = &tx_ring->tx_buffer_info[i];
}
--
2.13.3
^ permalink raw reply related
* [PATCH net-next v2 3/6] devlink: Adding num VFs per PF NVRAM config param
From: Steve Lin @ 2017-10-19 19:17 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, michael.chan, linville, gospo, steven.lin1
In-Reply-To: <1508440630-25830-1-git-send-email-steven.lin1@broadcom.com>
Adding DEVLINK_PERM_CONFIG_NUM_VF_PER_PF permanent config
parameter, which sets the number of VFs per PF in SR-IOV
mode. Value is permanent (stored in NVRAM), so becomes the
new default value for this device.
Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Acked-by: Andy Gospodarek <gospo@broadcom.com>
---
include/uapi/linux/devlink.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 2640203..09231e1 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -258,6 +258,7 @@ enum devlink_dpipe_header_id {
/* Permanent (NVRAM) config parameters */
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+ DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 0/6] Adding permanent config get/set to devlink
From: Steve Lin @ 2017-10-19 19:17 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, michael.chan, linville, gospo, steven.lin1
Changes since v1, based on the excellent feedback received:
* Implemented nested parameters correctly this time, I think.
* Submitting config get/set infrastructure separately from the
parameters themselves, and then submitting just the first four
parameters as separate patches. Once this approach is
accepted, I will add additional parameters, taking into
account comments received on them.
* Changed devlink_nl_sing_param_get/set to use _single_.
* Tried to make clear that all params using this command are
permanent / NVRAM settings, not transient.
* Split out the reorganization of bnxt driver to separate patch,
submitted to net-next earlier today.
* One non-change: The devices this change affects don't typically
have a separate 'asic' pci b/d/f versus per-port b/d/f; they
just have (typically multiple) b/d/f entities for each function
on the device. So, doesn't seem to me like splitting these
parameters into port vs. device params works here.
Adds a devlink command for getting & setting permanent
(persistent / NVRAM) device configuration parameters, and
enumerates the parameters as nested devlink attributes.
bnxt driver patch makes use of these new devlink cmds.
Steve Lin (6):
devlink: Add permanent config parameter get/set operations
devlink: Adding SR-IOV enablement NVRAM config param
devlink: Adding num VFs per PF NVRAM config param
devlink: Adding max PF MSI-X vectors NVRAM config param
devlink: Adding num MSI-X vectors per VF NVRAM config param
bnxt: Add devlink support for config get/set
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 245 +++++++++++++++++++++-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 +++++++++
include/net/devlink.h | 3 +
include/uapi/linux/devlink.h | 19 ++
net/core/devlink.c | 234 +++++++++++++++++++++
6 files changed, 612 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next v2 4/6] devlink: Adding max PF MSI-X vectors NVRAM config param
From: Steve Lin @ 2017-10-19 19:17 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, michael.chan, linville, gospo, steven.lin1
In-Reply-To: <1508440630-25830-1-git-send-email-steven.lin1@broadcom.com>
Adding DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT permanent config
parameter. Sets the maximum number of PF MSI-X (Message
Signaled Interrupts) vectors. Value is permanent (stored in
NVRAM), so becomes the new default value for this device.
Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Acked-by: Andy Gospodarek <gospo@broadcom.com>
---
include/uapi/linux/devlink.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 09231e1..8ad6c63 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -259,6 +259,7 @@ enum devlink_dpipe_header_id {
enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
+ DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 2/6] devlink: Adding SR-IOV enablement NVRAM config param
From: Steve Lin @ 2017-10-19 19:17 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, michael.chan, linville, gospo, steven.lin1
In-Reply-To: <1508440630-25830-1-git-send-email-steven.lin1@broadcom.com>
Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
parameter. If value is 1, SR-IOV is enabled. If value is 0,
SR-IOV is disabled on this device. Value is permanent (stored
in NVRAM), so becomes the new default value for this device.
Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Acked-by: Andy Gospodarek <gospo@broadcom.com>
---
include/uapi/linux/devlink.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 47cc584..2640203 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -255,4 +255,9 @@ enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_IPV6,
};
+/* Permanent (NVRAM) config parameters */
+enum devlink_perm_config_param {
+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
+};
+
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
From: Steve Lin @ 2017-10-19 19:17 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, michael.chan, linville, gospo, steven.lin1
In-Reply-To: <1508440630-25830-1-git-send-email-steven.lin1@broadcom.com>
Add support for permanent config parameter get/set commands. Used
for parameters held in NVRAM, persistent device configuration.
Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Acked-by: Andy Gospodarek <gospo@broadcom.com>
---
include/net/devlink.h | 3 +
include/uapi/linux/devlink.h | 11 ++
net/core/devlink.c | 234 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 248 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9654e1..bd64623 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -270,6 +270,9 @@ struct devlink_ops {
int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
int (*eswitch_encap_mode_get)(struct devlink *devlink, u8 *p_encap_mode);
int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
+ int (*perm_config_get)(struct devlink *devlink, u32 param, u32 *value);
+ int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
+ u8 *restart_reqd);
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 0cbca96..47cc584 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -70,6 +70,10 @@ enum devlink_command {
DEVLINK_CMD_DPIPE_HEADERS_GET,
DEVLINK_CMD_DPIPE_TABLE_COUNTERS_SET,
+ /* Permanent (NVRAM) device config get/set */
+ DEVLINK_CMD_PERM_CONFIG_GET,
+ DEVLINK_CMD_PERM_CONFIG_SET,
+
/* add new commands above here */
__DEVLINK_CMD_MAX,
DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -202,6 +206,13 @@ enum devlink_attr {
DEVLINK_ATTR_ESWITCH_ENCAP_MODE, /* u8 */
+ /* Permanent Configuration Parameters */
+ DEVLINK_ATTR_PERM_CONFIGS, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG, /* nested */
+ DEVLINK_ATTR_PERM_CONFIG_PARAMETER, /* u32 */
+ DEVLINK_ATTR_PERM_CONFIG_VALUE, /* u32 */
+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, /* u8 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7d430c1..c2cc7c6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1566,6 +1566,224 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb,
return 0;
}
+static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1];
+
+static int devlink_nl_single_param_get(struct sk_buff *msg,
+ struct devlink *devlink,
+ uint32_t param)
+{
+ u32 value;
+ int err;
+ const struct devlink_ops *ops = devlink->ops;
+ struct nlattr *param_attr;
+
+ err = ops->perm_config_get(devlink, param, &value);
+ if (err)
+ return err;
+
+ param_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
+ nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
+ nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, value);
+ nla_nest_end(msg, param_attr);
+
+ return 0;
+}
+
+static int devlink_nl_config_get_fill(struct sk_buff *msg,
+ struct devlink *devlink,
+ enum devlink_command cmd,
+ struct genl_info *info)
+{
+ void *hdr;
+ int err;
+ struct nlattr *attr;
+ int param_count = 0;
+ struct nlattr *cfgparam_attr;
+ int rem;
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
+ u32 param;
+
+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
+ &devlink_nl_family, 0, cmd);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto nla_msg_failure;
+ }
+
+ err = devlink_nl_put_handle(msg, devlink);
+ if (err)
+ goto nla_put_failure;
+
+ if (!info->attrs[DEVLINK_ATTR_PERM_CONFIGS]) {
+ /* No configuration parameters */
+ goto nla_put_failure;
+ }
+
+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIGS);
+
+ nla_for_each_nested(attr, info->attrs[DEVLINK_ATTR_PERM_CONFIGS],
+ rem) {
+ err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
+ devlink_nl_policy, NULL);
+ if (err)
+ goto nla_nest_failure;
+ if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER])
+ continue;
+
+ param = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
+ err = devlink_nl_single_param_get(msg, devlink, param);
+ if (err)
+ goto nla_nest_failure;
+ param_count++;
+ }
+
+ nla_nest_end(msg, cfgparam_attr);
+
+ genlmsg_end(msg, hdr);
+ return 0;
+
+nla_nest_failure:
+ nla_nest_cancel(msg, cfgparam_attr);
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+nla_msg_failure:
+ return err;
+}
+
+static int devlink_nl_cmd_perm_config_get_doit(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct devlink *devlink = info->user_ptr[0];
+ struct sk_buff *msg;
+ int err;
+
+ if (!devlink->ops || !devlink->ops->perm_config_get)
+ return -EOPNOTSUPP;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ err = devlink_nl_config_get_fill(msg, devlink,
+ DEVLINK_CMD_PERM_CONFIG_GET, info);
+
+ if (err) {
+ nlmsg_free(msg);
+ return err;
+ }
+
+ return genlmsg_reply(msg, info);
+}
+
+static int devlink_nl_single_param_set(struct sk_buff *msg,
+ struct devlink *devlink,
+ u32 param, u32 value)
+{
+ u32 orig_value;
+ u8 need_restart;
+ int err;
+ const struct devlink_ops *ops = devlink->ops;
+ struct nlattr *cfgparam_attr;
+
+ /* First get current value of parameter */
+ err = ops->perm_config_get(devlink, param, &orig_value);
+ if (err)
+ return err;
+
+ /* Now set parameter */
+ err = ops->perm_config_set(devlink, param, value, &need_restart);
+ if (err)
+ return err;
+
+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
+ /* Update restart reqd - if any param needs restart, should be set */
+ if (need_restart)
+ err = nla_put_u8(msg,
+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);
+
+ /* Since set was successful, write attr back to msg with orig val */
+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, orig_value);
+
+ nla_nest_end(msg, cfgparam_attr);
+
+ return 0;
+}
+
+static int devlink_nl_cmd_perm_config_set_doit(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct devlink *devlink = info->user_ptr[0];
+ struct sk_buff *msg;
+ void *hdr;
+ struct nlattr *attr;
+ int rem;
+ int err;
+ u8 restart_reqd = 0;
+ struct nlattr *cfgparam_attr;
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
+ u32 param;
+ u32 value;
+
+ if (!devlink->ops || !devlink->ops->perm_config_get ||
+ !devlink->ops->perm_config_set)
+ return -EOPNOTSUPP;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
+ &devlink_nl_family, 0, DEVLINK_CMD_PERM_CONFIG_SET);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto nla_msg_failure;
+ }
+
+ err = devlink_nl_put_handle(msg, devlink);
+ if (err)
+ goto nla_put_failure;
+
+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIGS);
+
+ nla_for_each_nested(attr, info->attrs[DEVLINK_ATTR_PERM_CONFIGS], rem) {
+ err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
+ devlink_nl_policy, NULL);
+ if (err)
+ goto nla_nest_failure;
+
+ if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER] ||
+ !tb[DEVLINK_ATTR_PERM_CONFIG_VALUE])
+ continue;
+
+ param = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
+ value = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_VALUE]);
+ err = devlink_nl_single_param_set(msg, devlink, param,
+ value);
+ if (err)
+ goto nla_nest_failure;
+ }
+
+ nla_nest_end(msg, cfgparam_attr);
+
+ if (restart_reqd) {
+ err = nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED,
+ restart_reqd);
+ if (err)
+ goto nla_put_failure;
+ }
+
+ genlmsg_end(msg, hdr);
+ return genlmsg_reply(msg, info);
+
+nla_nest_failure:
+ nla_nest_cancel(msg, cfgparam_attr);
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+nla_msg_failure:
+ return err;
+}
+
int devlink_dpipe_match_put(struct sk_buff *skb,
struct devlink_dpipe_match *match)
{
@@ -2291,6 +2509,8 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_ESWITCH_ENCAP_MODE] = { .type = NLA_U8 },
[DEVLINK_ATTR_DPIPE_TABLE_NAME] = { .type = NLA_NUL_STRING },
[DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED] = { .type = NLA_U8 },
+ [DEVLINK_ATTR_PERM_CONFIG_PARAMETER] = { .type = NLA_U32 },
+ [DEVLINK_ATTR_PERM_CONFIG_VALUE] = { .type = NLA_U32 },
};
static const struct genl_ops devlink_nl_ops[] = {
@@ -2451,6 +2671,20 @@ static const struct genl_ops devlink_nl_ops[] = {
.flags = GENL_ADMIN_PERM,
.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
},
+ {
+ .cmd = DEVLINK_CMD_PERM_CONFIG_GET,
+ .doit = devlink_nl_cmd_perm_config_get_doit,
+ .policy = devlink_nl_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+ },
+ {
+ .cmd = DEVLINK_CMD_PERM_CONFIG_SET,
+ .doit = devlink_nl_cmd_perm_config_set_doit,
+ .policy = devlink_nl_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+ },
};
static struct genl_family devlink_nl_family __ro_after_init = {
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Steve Lin @ 2017-10-19 19:17 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, michael.chan, linville, gospo, steven.lin1
In-Reply-To: <1508440630-25830-1-git-send-email-steven.lin1@broadcom.com>
Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent config
parameter. Defines number of MSI-X vectors allocated per VF.
Value is permanent (stored in NVRAM), so becomes the new default
value for this device.
Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Acked-by: Andy Gospodarek <gospo@broadcom.com>
---
include/uapi/linux/devlink.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 8ad6c63..ef163b6 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -260,6 +260,7 @@ enum devlink_perm_config_param {
DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
+ DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 6/6] bnxt: Add devlink support for config get/set
From: Steve Lin @ 2017-10-19 19:17 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, michael.chan, linville, gospo, steven.lin1
In-Reply-To: <1508440630-25830-1-git-send-email-steven.lin1@broadcom.com>
Implements get and set of configuration parameters using new devlink
config get/set API.
Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Acked-by: Andy Gospodarek <gospo@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 245 +++++++++++++++++++++-
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 17 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 100 +++++++++
3 files changed, 356 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index f3f6aa8..88a1f1d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -14,11 +14,244 @@
#include "bnxt_vfr.h"
#include "bnxt_devlink.h"
-static const struct devlink_ops bnxt_dl_ops = {
+struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
+ {DEVLINK_PERM_CONFIG_SRIOV_ENABLED, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 1, 401},
+ {DEVLINK_PERM_CONFIG_NUM_VF_PER_PF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 8, 404},
+ {DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT, BNXT_DRV_PF,
+ BNXT_DRV_APPL_SHARED, 10, 108},
+ {DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF, BNXT_DRV_PF,
+ BNXT_DRV_APPL_FUNCTION, 10, 406},
+};
+
+#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
+
+static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
+ void *buf, int size)
+{
+ struct hwrm_nvm_get_variable_input req = {0};
+ void *dest_data_addr = NULL;
+ dma_addr_t dest_data_dma_addr;
+ int rc;
+ int bytesize;
+
+ bytesize = (size + 7) / 8;
+ dest_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &dest_data_dma_addr, GFP_KERNEL);
+ if (!dest_data_addr) {
+ netdev_err(bp->dev, "dma_alloc_coherent failure\n");
+ return -ENOMEM;
+ }
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
+ req.dest_data_addr = cpu_to_le64(dest_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ memcpy(buf, dest_data_addr, bytesize);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, dest_data_addr,
+ dest_data_dma_addr);
+
+ return rc;
+}
+
+static int bnxt_nvm_write(struct bnxt *bp, int nvm_param, int idx,
+ const void *buf, int size)
+{
+ struct hwrm_nvm_set_variable_input req = {0};
+ void *src_data_addr = NULL;
+ dma_addr_t src_data_dma_addr;
+ int rc;
+ int bytesize;
+
+ bytesize = (size + 7) / 8;
+
+ src_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
+ &src_data_dma_addr, GFP_KERNEL);
+ if (!src_data_addr) {
+ netdev_err(bp->dev, "dma_alloc_coherent failure\n");
+ return -ENOMEM;
+ }
+
+ memcpy(src_data_addr, buf, bytesize);
+
+ bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_SET_VARIABLE, -1, -1);
+ req.src_data_addr = cpu_to_le64(src_data_dma_addr);
+ req.data_len = cpu_to_le16(size);
+ req.option_num = cpu_to_le16(nvm_param);
+ req.index_0 = cpu_to_le16(idx);
+ if (idx != 0)
+ req.dimensions = cpu_to_le16(1);
+
+ rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+
+ dma_free_coherent(&bp->pdev->dev, bytesize, src_data_addr,
+ src_data_dma_addr);
+
+ return 0;
+}
+
+static int bnxt_dl_perm_config_set(struct devlink *devlink,
+ u32 param, u32 value, u8 *restart_reqd)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+ int i;
+ int idx = 0;
+ void *data;
+ int ret = 0;
+ u32 bytesize;
+ struct bnxt_drv_cfgparam *entry;
+
+ *restart_reqd = 0;
+
+ /* Find parameter in table */
+ for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
+ if (param == bnxt_drv_cfgparam_list[i].param) {
+ entry = &bnxt_drv_cfgparam_list[i];
+ break;
+ }
+ }
+
+ /* Not found */
+ if (i == BNXT_NUM_DRV_CFGPARAM)
+ return -EINVAL;
+
+ /* Check to see if this func type can access variable */
+ if (BNXT_PF(bp) && !(entry->func & BNXT_DRV_PF))
+ return -EOPNOTSUPP;
+ if (BNXT_VF(bp) && !(entry->func & BNXT_DRV_VF))
+ return -EOPNOTSUPP;
+
+ /* If parameter is per port or function, compute index */
+ if (entry->appl == BNXT_DRV_APPL_PORT) {
+ idx = bp->pf.port_id;
+ } else if (entry->appl == BNXT_DRV_APPL_FUNCTION) {
+ if (BNXT_PF(bp))
+ idx = bp->pf.fw_fid - BNXT_FIRST_PF_FID;
+#ifdef CONFIG_BNXT_SRIOV
+ else
+ idx = bp->vf.fw_fid - BNXT_FIRST_VF_FID;
+#endif /* CONFIG_BNXT_SRIOV */
+ }
+
+ bytesize = (entry->bitlength + 7) / 8;
+ data = kmalloc(bytesize, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ if (bytesize == 1) {
+ u8 val8 = (value & 0xff);
+
+ memcpy(data, &val8, sizeof(u8));
+ } else if (bytesize == 2) {
+ u16 val16 = (value & 0xffff);
+
+ memcpy(data, &val16, sizeof(u16));
+ } else {
+ memcpy(data, &value, sizeof(u32));
+ }
+
+ ret = bnxt_nvm_write(bp, entry->nvm_param, idx, data,
+ entry->bitlength);
+
+ /* Restart required for all nvm parameter writes */
+ *restart_reqd = 1;
+
+ kfree(data);
+
+ return ret;
+}
+
+static int bnxt_dl_perm_config_get(struct devlink *devlink, u32 param,
+ u32 *value)
+{
+ struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
+
+ int i;
+ int idx = 0;
+ void *data;
+ int ret = 0;
+ u32 bytesize;
+ struct bnxt_drv_cfgparam *entry;
+
+ /* Find parameter in table */
+ for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
+ if (param == bnxt_drv_cfgparam_list[i].param) {
+ entry = &bnxt_drv_cfgparam_list[i];
+ break;
+ }
+ }
+
+ /* Not found */
+ if (i == BNXT_NUM_DRV_CFGPARAM)
+ return -EINVAL;
+
+ /* Check to see if this func type can access variable */
+ if (BNXT_PF(bp) && !(entry->func & BNXT_DRV_PF))
+ return -EOPNOTSUPP;
+ if (BNXT_VF(bp) && !(entry->func & BNXT_DRV_VF))
+ return -EOPNOTSUPP;
+
+ /* If parameter is per port or function, compute index */
+ if (entry->appl == BNXT_DRV_APPL_PORT) {
+ idx = bp->pf.port_id;
+ } else if (entry->appl == BNXT_DRV_APPL_FUNCTION) {
+ if (BNXT_PF(bp))
+ idx = bp->pf.fw_fid - BNXT_FIRST_PF_FID;
+#ifdef CONFIG_BNXT_SRIOV
+ else
+ idx = bp->vf.fw_fid - BNXT_FIRST_VF_FID;
+#endif /* CONFIG_BNXT_SRIOV */
+ }
+
+ /* Allocate space, retrieve value, and copy to result */
+ bytesize = (entry->bitlength + 7) / 8;
+ data = kmalloc(bytesize, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ ret = bnxt_nvm_read(bp, entry->nvm_param, idx, data, entry->bitlength);
+
+ if (ret) {
+ kfree(data);
+ return ret;
+ }
+
+ if (bytesize == 1) {
+ u8 val;
+
+ memcpy(&val, data, sizeof(u8));
+ *value = val;
+ } else if (bytesize == 2) {
+ u16 val;
+
+ memcpy(&val, data, sizeof(u16));
+ *value = val;
+ } else {
+ u32 val;
+
+ memcpy(&val, data, sizeof(u32));
+ *value = val;
+ }
+
+ kfree(data);
+
+ return 0;
+}
+
+static struct devlink_ops bnxt_dl_ops = {
#ifdef CONFIG_BNXT_SRIOV
.eswitch_mode_set = bnxt_dl_eswitch_mode_set,
.eswitch_mode_get = bnxt_dl_eswitch_mode_get,
#endif /* CONFIG_BNXT_SRIOV */
+ .perm_config_get = bnxt_dl_perm_config_get,
+ .perm_config_set = bnxt_dl_perm_config_set,
};
int bnxt_dl_register(struct bnxt *bp)
@@ -26,12 +259,12 @@ int bnxt_dl_register(struct bnxt *bp)
struct devlink *dl;
int rc;
- if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
- return 0;
-
- if (bp->hwrm_spec_code < 0x10800) {
+ if ((!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV)) ||
+ bp->hwrm_spec_code < 0x10800) {
+ /* eswitch switchdev mode not supported */
+ bnxt_dl_ops.eswitch_mode_set = NULL;
+ bnxt_dl_ops.eswitch_mode_get = NULL;
netdev_warn(bp->dev, "Firmware does not support SR-IOV E-Switch SWITCHDEV mode.\n");
- return -ENOTSUPP;
}
dl = devlink_alloc(&bnxt_dl_ops, sizeof(struct bnxt_dl));
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index e92a35d..d843a81 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -10,6 +10,23 @@
#ifndef BNXT_DEVLINK_H
#define BNXT_DEVLINK_H
+#define BNXT_DRV_PF 1
+#define BNXT_DRV_VF 2
+
+enum bnxt_drv_appl {
+ BNXT_DRV_APPL_SHARED,
+ BNXT_DRV_APPL_PORT,
+ BNXT_DRV_APPL_FUNCTION
+};
+
+struct bnxt_drv_cfgparam {
+ enum devlink_perm_config_param param;
+ u8 func; /* BNXT_DRV_PF | BNXT_DRV_VF */
+ enum bnxt_drv_appl appl; /* applicability (shared, func, port) */
+ u32 bitlength; /* length, in bits */
+ u32 nvm_param;
+};
+
/* Struct to hold housekeeping info needed by devlink interface */
struct bnxt_dl {
struct bnxt *bp; /* back ptr to the controlling dev */
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
index cb04cc7..8c25731 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
@@ -5676,6 +5676,106 @@ struct hwrm_nvm_install_update_cmd_err {
u8 unused_0[7];
};
+/* hwrm_nvm_get_variable */
+/* Input (40 bytes) */
+struct hwrm_nvm_get_variable_input {
+ __le16 req_type;
+ __le16 cmpl_ring;
+ __le16 seq_id;
+ __le16 target_id;
+ __le64 resp_addr;
+ __le64 dest_data_addr;
+ __le16 data_len;
+ __le16 option_num;
+ #define NVM_GET_VARIABLE_REQ_OPTION_NUM_RSVD_0 0x0UL
+ #define NVM_GET_VARIABLE_REQ_OPTION_NUM_RSVD_FFFF 0xffffUL
+ __le16 dimensions;
+ __le16 index_0;
+ __le16 index_1;
+ __le16 index_2;
+ __le16 index_3;
+ u8 flags;
+ #define NVM_GET_VARIABLE_REQ_FLAGS_FACTORY_DFLT 0x1UL
+ u8 unused_0;
+};
+
+/* Output (16 bytes) */
+struct hwrm_nvm_get_variable_output {
+ __le16 error_code;
+ __le16 req_type;
+ __le16 seq_id;
+ __le16 resp_len;
+ __le16 data_len;
+ __le16 option_num;
+ #define NVM_GET_VARIABLE_RESP_OPTION_NUM_RSVD_0 0x0UL
+ #define NVM_GET_VARIABLE_RESP_OPTION_NUM_RSVD_FFFF 0xffffUL
+ u8 unused_0;
+ u8 unused_1;
+ u8 unused_2;
+ u8 valid;
+};
+
+/* Command specific Error Codes (8 bytes) */
+struct hwrm_nvm_get_variable_cmd_err {
+ u8 code;
+ #define NVM_GET_VARIABLE_CMD_ERR_CODE_UNKNOWN 0x0UL
+ #define NVM_GET_VARIABLE_CMD_ERR_CODE_VAR_NOT_EXIST 0x1UL
+ #define NVM_GET_VARIABLE_CMD_ERR_CODE_CORRUPT_VAR 0x2UL
+ #define NVM_GET_VARIABLE_CMD_ERR_CODE_LEN_TOO_SHORT 0x3UL
+ u8 unused_0[7];
+};
+
+/* hwrm_nvm_set_variable */
+/* Input (40 bytes) */
+struct hwrm_nvm_set_variable_input {
+ __le16 req_type;
+ __le16 cmpl_ring;
+ __le16 seq_id;
+ __le16 target_id;
+ __le64 resp_addr;
+ __le64 src_data_addr;
+ __le16 data_len;
+ __le16 option_num;
+ #define NVM_SET_VARIABLE_REQ_OPTION_NUM_RSVD_0 0x0UL
+ #define NVM_SET_VARIABLE_REQ_OPTION_NUM_RSVD_FFFF 0xffffUL
+ __le16 dimensions;
+ __le16 index_0;
+ __le16 index_1;
+ __le16 index_2;
+ __le16 index_3;
+ u8 flags;
+ #define NVM_SET_VARIABLE_REQ_FLAGS_FORCE_FLUSH 0x1UL
+ #define NVM_SET_VARIABLE_REQ_FLAGS_ENCRYPT_MODE_MASK 0xeUL
+ #define NVM_SET_VARIABLE_REQ_FLAGS_ENCRYPT_MODE_SFT 1
+ #define NVM_SET_VARIABLE_REQ_FLAGS_ENCRYPT_MODE_NONE (0x0UL << 1)
+ #define NVM_SET_VARIABLE_REQ_FLAGS_ENCRYPT_MODE_HMAC_SHA1 (0x1UL << 1)
+ #define NVM_SET_VARIABLE_REQ_FLAGS_ENCRYPT_MODE_LAST \
+ NVM_SET_VARIABLE_REQ_FLAGS_ENCRYPT_MODE_HMAC_SHA1
+ u8 unused_0;
+};
+
+/* Output (16 bytes) */
+struct hwrm_nvm_set_variable_output {
+ __le16 error_code;
+ __le16 req_type;
+ __le16 seq_id;
+ __le16 resp_len;
+ __le32 unused_0;
+ u8 unused_1;
+ u8 unused_2;
+ u8 unused_3;
+ u8 valid;
+};
+
+/* Command specific Error Codes (8 bytes) */
+struct hwrm_nvm_set_variable_cmd_err {
+ u8 code;
+ #define NVM_SET_VARIABLE_CMD_ERR_CODE_UNKNOWN 0x0UL
+ #define NVM_SET_VARIABLE_CMD_ERR_CODE_VAR_NOT_EXIST 0x1UL
+ #define NVM_SET_VARIABLE_CMD_ERR_CODE_CORRUPT_VAR 0x2UL
+ u8 unused_0[7];
+};
+
/* hwrm_selftest_qlist */
/* Input (16 bytes) */
struct hwrm_selftest_qlist_input {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] wanxl: use m68k-linux-gnu-as if available
From: Adam Borowski @ 2017-10-19 19:31 UTC (permalink / raw)
To: Krzysztof Halasa; +Cc: David Miller, netdev
In-Reply-To: <m3y3o7nj7m.fsf@pm.waw.pl>
On Thu, Oct 19, 2017 at 07:25:17PM +0200, Krzysztof Halasa wrote:
> David Miller <davem@davemloft.net> writes:
>
> > We don't even know if whatever "as68k" is would be the same thing
> > as GNU as and generate the same binaries.
>
> It's GNU as, likewise ld68k, though I have no idea if recent versions
> would compile/link the firmware (correctly). This is 15+ years old.
Thus, we know that:
* this patch can't possibly make things worse: it merely makes compilation
succeed on systems (like Debian + derivatives) that name m68k GNU as
m68k-linux-gnu-as rather than as68k
* modern versions of "rose by any other name" (GNU as + ld) produce an
output that's not byte-to-byte identical. This would be understandable
for a compiler, spells trouble for an assembler. It's possible the modern
interpretation is still valid, but if it's not, the current rule to
rebuild the firmware has bitrotten (needs to pass some options to as?)
> I don't have opinion on the patch.
Do you still have access to such hardware? Could you test it?
Because two scenarios are possible:
*) the build still works (in which case my patch fixes randconfigs on
Debian)
*) it does not, and you have no resources to fix it. In which case, it
would be better to disable the automatic rule and add a comment "to
build this firmware, dig an ancient version of as+ld". Or perhaps
remove this driver as unmaintaineable.
Meow!
--
⢀⣴⠾⠻⢶⣦⠀ Laws we want back: Poland, Dz.U. 1921 nr.30 poz.177 (also Dz.U.
⣾⠁⢰⠒⠀⣿⡁ 1920 nr.11 poz.61): Art.2: An official, guilty of accepting a gift
⢿⡄⠘⠷⠚⠋⠀ or another material benefit, or a promise thereof, [in matters
⠈⠳⣄⠀⠀⠀⠀ relevant to duties], shall be punished by death by shooting.
^ permalink raw reply
* Re: [PATCH net-next v2 2/6] devlink: Adding SR-IOV enablement NVRAM config param
From: Jiri Pirko @ 2017-10-19 19:33 UTC (permalink / raw)
To: Steve Lin; +Cc: netdev, jiri, davem, michael.chan, linville, gospo
In-Reply-To: <1508440630-25830-3-git-send-email-steven.lin1@broadcom.com>
Thu, Oct 19, 2017 at 09:17:06PM CEST, steven.lin1@broadcom.com wrote:
>Adding DEVLINK_PERM_CONFIG_SRIOV_ENABLED permanent config
>parameter. If value is 1, SR-IOV is enabled. If value is 0,
>SR-IOV is disabled on this device. Value is permanent (stored
>in NVRAM), so becomes the new default value for this device.
>
>Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>Acked-by: Andy Gospodarek <gospo@broadcom.com>
>---
> include/uapi/linux/devlink.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 47cc584..2640203 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -255,4 +255,9 @@ enum devlink_dpipe_header_id {
> DEVLINK_DPIPE_HEADER_IPV6,
> };
>
>+/* Permanent (NVRAM) config parameters */
We need the decription here in the header as well. Commit message alone
is no good for this.
Also, there should not be mention of "NVRAM". It is up to the device
implementation where is stores the value.
>+enum devlink_perm_config_param {
>+ DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>+};
>+
> #endif /* _UAPI_LINUX_DEVLINK_H_ */
>--
>2.7.4
>
^ permalink raw reply
* Re: [PATCH net-next v2 6/6] bnxt: Add devlink support for config get/set
From: Jiri Pirko @ 2017-10-19 19:35 UTC (permalink / raw)
To: Steve Lin; +Cc: netdev, jiri, davem, michael.chan, linville, gospo
In-Reply-To: <1508440630-25830-7-git-send-email-steven.lin1@broadcom.com>
Thu, Oct 19, 2017 at 09:17:10PM CEST, steven.lin1@broadcom.com wrote:
>Implements get and set of configuration parameters using new devlink
>config get/set API.
Please split this patch too. One to introduce the infra, one per each
config option.
^ permalink raw reply
* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Mario Hüttel @ 2017-10-19 19:54 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde, Sekhar Nori,
Franklin S Cooper Jr, Yang, Wenyou, wg, quentin.schulz, edumazet,
linux-can, netdev, linux-kernel
Cc: Wenyou Yang, Dong Aisheng, Quadros, Roger
In-Reply-To: <f651ffd4-826c-ee20-aebf-e3c2c1b89ecf@hartkopp.net>
[-- Attachment #1.1: Type: text/plain, Size: 3128 bytes --]
On 10/19/2017 08:35 PM, Oliver Hartkopp wrote:
> Hi Marc,
>
> On 10/19/2017 01:26 PM, Marc Kleine-Budde wrote:
>> On 10/19/2017 01:14 PM, Oliver Hartkopp wrote:
>>>>>>> Since we have a netlink socket interface to configure sample
>>>>>>> point, I
>>>>>>> wonder if that should be extended to configure SSP too (or at
>>>>>>> least the
>>>>>>> offset part of SSP)?
>>>
>>> +1 too
>>
>> The struct can_bittiming in defined in uapi, so we have to keep ABI
>> compatibility in mind.
>>
>
> Oh, this is fortunately NO problem ;-)
>
> struct can_bittiming {
> __u32 bitrate; /* Bit-rate in bits/second */
> __u32 sample_point; /* Sample point in one-tenth of a
> percent */
> __u32 tq; /* Time quanta (TQ) in nanoseconds */
> __u32 prop_seg; /* Propagation segment in TQs */
> __u32 phase_seg1; /* Phase buffer segment 1 in TQs */
> __u32 phase_seg2; /* Phase buffer segment 2 in TQs */
> __u32 sjw; /* Synchronisation jump width in TQs */
> __u32 brp; /* Bit-rate prescaler */
> };
>
> So we have two of these: One for the arbitration bitrate and one
> sample_point for the data bitrate -> the 'secondary' SP -> SSP
>
> :-)
>
> We already have this 'dsample-point' implemented in the ip tool:
>
> $ ip link set vcan0 type can help
> Usage: ip link set DEVICE type can
> [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
> [ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1
> phase-seg2 PHASE-SEG2 [ sjw SJW ] ]
>
> [ dbitrate BITRATE [ dsample-point SAMPLE-POINT] ] | <<-- here!
> [ dtq TQ dprop-seg PROP_SEG dphase-seg1 PHASE-SEG1
> dphase-seg2 PHASE-SEG2 [ dsjw SJW ] ]
>
> But AFAIK m_can is not using that value in m_can_set_bittiming().
>
Actually I need some clarification. The sample point of the can core is
between the two time segments.
I always thought that the "sample point" options of the ip tool are used
in the internal
calculation of the two timing segments and is therefore no individual value.
>>>> If good default values are transceiver and board specific, they can go
>>>> into the DT. We need a generic (this means driver agnostic) binding
>>>> for
>>>> this. If this table needs to be tweaked for special purpose, then
>>>> we can
>>>> add a netlink interface for this as well. >
>>>> Comments?
>>>
>>> By now we calculate reasonable default values (e.g. for SP and SJW),
>>> you
>>> can override by setting alternative values via netlink configuration.
>>>
>>> I would tend to stay on this approach and not hide these things in
>>> DTs -
>>> just because of someone wants to initialize his specific interface
>>> 'easier'.
>>
>> If the values are not board specific, then it makes no sense to put them
>> into the DT.
>
> When they are NOT(?) board specific?
>
> Thinking about non-SoC CAN adapters with PCI and USB pushing the SSP
> to the DT looks wrong to me.
>
> Best,
> Oliver
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] igb: Fix TX map failure path
From: Alexander Duyck @ 2017-10-19 19:58 UTC (permalink / raw)
To: Jean-Philippe Brucker; +Cc: Jeff Kirsher, intel-wired-lan, Netdev
In-Reply-To: <20171019190736.2770-1-jean-philippe.brucker@arm.com>
On Thu, Oct 19, 2017 at 12:07 PM, Jean-Philippe Brucker
<jean-philippe.brucker@arm.com> wrote:
> When the driver cannot map a TX buffer, instead of rolling back
> gracefully and retrying later, we currently get a panic:
>
> [ 159.885994] igb 0000:00:00.0: TX DMA map failed
> [ 159.886588] Unable to handle kernel paging request at virtual address ffff00000a08c7a8
> ...
> [ 159.897031] PC is at igb_xmit_frame_ring+0x9c8/0xcb8
>
> Fix the erroneous test that leads to this situation.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index fd4a46b03cc8..ea69af267d63 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -5326,7 +5326,7 @@ static int igb_tx_map(struct igb_ring *tx_ring,
> DMA_TO_DEVICE);
> dma_unmap_len_set(tx_buffer, len, 0);
>
> - if (i--)
> + if (i-- == 0)
> i += tx_ring->count;
> tx_buffer = &tx_ring->tx_buffer_info[i];
> }
The fix looks fine to me, though I will probably want to go back and
update the code to simplify this since I realize part of the issue is
that there is a bunch of unneeded complexity I introduced here. It
looks like we have the same problem in ixgbe that needs to be
addressed as well.
Fixes: 7cc6fd4c60f2 ("igb: Don't bother clearing Tx buffer_info in
igb_clean_tx_ring")
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply
* Re: RFC(v2): Audit Kernel Container IDs
From: Richard Guy Briggs @ 2017-10-19 19:57 UTC (permalink / raw)
To: Steve Grubb
Cc: cgroups, Linux Containers, Linux API, Linux Audit, Linux FS Devel,
Linux Kernel, Linux Network Development, Simo Sorce,
Carlos O'Donell, Aristeu Rozanski, David Howells,
Eric W. Biederman, Eric Paris, jlayton, Andy Lutomirski, mszeredi,
Paul Moore, Serge E. Hallyn, trondmy, Al Viro
In-Reply-To: <2307769.VGpzlLa4Dp@x2>
On 2017-10-12 15:45, Steve Grubb wrote:
> On Thursday, October 12, 2017 10:14:00 AM EDT Richard Guy Briggs wrote:
> > Containers are a userspace concept. The kernel knows nothing of them.
> >
> > The Linux audit system needs a way to be able to track the container
> > provenance of events and actions. Audit needs the kernel's help to do
> > this.
> >
> > Since the concept of a container is entirely a userspace concept, a
> > registration from the userspace container orchestration system initiates
> > this. This will define a point in time and a set of resources
> > associated with a particular container with an audit container ID.
>
> The requirements for common criteria around containers should be very closely
> modeled on the requirements for virtualization. It would be the container
> manager that is responsible for logging the resource assignment events.
I suspect we are in violent agreement here.
> > The registration is a pseudo filesystem (proc, since PID tree already
> > exists) write of a u8[16] UUID representing the container ID to a file
> > representing a process that will become the first process in a new
> > container. This write might place restrictions on mount namespaces
> > required to define a container, or at least careful checking of
> > namespaces in the kernel to verify permissions of the orchestrator so it
> > can't change its own container ID. A bind mount of nsfs may be
> > necessary in the container orchestrator's mntNS.
> > Note: Use a 128-bit scalar rather than a string to make compares faster
> > and simpler.
> >
> > Require a new CAP_CONTAINER_ADMIN to be able to carry out the
> > registration.
>
> Wouldn't CAP_AUDIT_WRITE be sufficient? After all, this is for auditing.
No, because then any process with that capability (vsftpd) could change
its own container ID. This is discussed more in other parts of the
thread...
> > At that time, record the target container's user-supplied
> > container identifier along with the target container's first process
> > (which may become the target container's "init" process) process ID
> > (referenced from the initial PID namespace), all namespace IDs (in the
> > form of a nsfs device number and inode number tuple) in a new auxilliary
> > record AUDIT_CONTAINER with a qualifying op=$action field.
>
> This would be in addition to the normal audit fields.
It was intended that this be an auxilliary record, but this issue is
being debated in threads about other upstream issues currently so I
won't cover that here.
> > Issue a new auxilliary record AUDIT_CONTAINER_INFO for each valid
> > container ID present on an auditable action or event.
> >
> > Forked and cloned processes inherit their parent's container ID,
> > referenced in the process' task_struct.
> >
> > Mimic setns(2) and return an error if the process has already initiated
> > threading or forked since this registration should happen before the
> > process execution is started by the orchestrator and hence should not
> > yet have any threads or children. If this is deemed overly restrictive,
> > switch all threads and children to the new containerID.
> >
> > Trust the orchestrator to judiciously use and restrict CAP_CONTAINER_ADMIN.
> >
> > Log the creation of every namespace, inheriting/adding its spawning
> > process' containerID(s), if applicable. Include the spawning and
> > spawned namespace IDs (device and inode number tuples).
> > [AUDIT_NS_CREATE, AUDIT_NS_DESTROY] [clone(2), unshare(2), setns(2)]
> > Note: At this point it appears only network namespaces may need to track
> > container IDs apart from processes since incoming packets may cause an
> > auditable event before being associated with a process.
> >
> > Log the destruction of every namespace when it is no longer used by any
> > process, include the namespace IDs (device and inode number tuples).
> > [AUDIT_NS_DESTROY] [process exit, unshare(2), setns(2)]
>
> In the virtualization requirements, we only log removal of resources when
> something is removed by intention. If the VM shuts down, the manager issues a
> VIRT_CONTROL stop event and the user space utilities knows this means all
> resources have been unassigned.
Ok, this assumes the orchestrator is waiting on that child process (and
that it is in turn waiting on all its children) so it knows when that
job has exited naturally or errored out. I don't know if there is any
consensus or best practice with orchestrators out there now. The kernel
should know, so it seemed reasonable to report what was known. Besides,
in this case, I was talking specifically about namespace creation and
destruction rather than containers.
> > Issue a new auxilliary record AUDIT_NS_CHANGE listing (opt: op=$action)
> > the parent and child namespace IDs for any changes to a process'
> > namespaces. [setns(2)]
> > Note: It may be possible to combine AUDIT_NS_* record formats and
> > distinguish them with an op=$action field depending on the fields
> > required for each message type.
> >
> > When a container ceases to exist because the last process in that
> > container has exited and hence the last namespace has been destroyed and
> > its refcount dropping to zero, log the fact.
> > (This latter is likely needed for certification accountability.) A
> > container object may need a list of processes and/or namespaces.
> >
> > A namespace cannot directly migrate from one container to another but
> > could be assigned to a newly spawned container. A namespace can be
> > moved from one container to another indirectly by having that namespace
> > used in a second process in another container and then ending all the
> > processes in the first container.
>
> I'm thinking that there needs to be a clear delineation between what the
> container manager is responsible for and what the kernel needs to do. The
> kernel needs the registration system and to associate an identifier with
> events inside the container.
Agreed this needs to be defined much better than it is.
> But would the container manager be mostly responsible for auditing the events
> described here:
>
> https://github.com/linux-audit/audit-documentation/wiki/SPEC-Virtualization-Manager-Guest-Lifecycle-Events
I'm having trouble fitting all these events into the container model,
but recognize its importance in continuing to try to do so or to be able
to justify deviations from this SPEC.
> Also, we can already audit exit, unshare, setns, and clone. If the kernel just
> sticks the identifier on them, isn't that sufficient?
I think this last one is incomplete without a way to identify the
namespaces involved.
> -Steve
>
> > (v2)
> > - switch from u64 to u128 UUID
> > - switch from "signal" and "trigger" to "register"
> > - restrict registration to single process or force all threads and children
> > into same container
> >
> > - RGB
- RGB
^ permalink raw reply
* (unknown),
From: pooks005 @ 2017-10-19 20:10 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 494911192325944.zip --]
[-- Type: application/zip, Size: 43595 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] ipv6: start fib6 gc on RTF_CACHE dst creation
From: Wei Wang @ 2017-10-19 20:13 UTC (permalink / raw)
To: Paolo Abeni
Cc: Linux Kernel Network Developers, David S. Miller, Eric Dumazet,
Hannes Frederic Sowa, Martin KaFai Lau
In-Reply-To: <f699f29b3fb7675fdfe5d85ab39e153c147c12f8.1508421311.git.pabeni@redhat.com>
On Thu, Oct 19, 2017 at 7:07 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> After the commit 2b760fcf5cfb ("ipv6: hook up exception table
> to store dst cache"), the fib6 gc is not started after the
> creation of a RTF_CACHE via a redirect or pmtu update, since
> fib6_add() isn't invoked anymore for such dsts.
>
> We need the fib6 gc to run periodically to clean the RTF_CACHE,
> or the dst will stay there forever.
>
> Fix it by explicitly calling fib6_force_start_gc() on successful
> exception creation. gc_args->more accounting will ensure that
> the gc timer will run for whatever time needed to properly
> clean the table.
>
> v2 -> v3:
> - clarified the commit message
>
> Fixes: 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
Acked-by: Wei Wang <weiwan@google.com>
> net/ipv6/route.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 01a103c23a6c..5c27313803d2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1340,8 +1340,10 @@ static int rt6_insert_exception(struct rt6_info *nrt,
> spin_unlock_bh(&rt6_exception_lock);
>
> /* Update fn->fn_sernum to invalidate all cached dst */
> - if (!err)
> + if (!err) {
> fib6_update_sernum(ort);
> + fib6_force_start_gc(net);
> + }
>
> return err;
> }
> --
> 2.13.6
>
^ permalink raw reply
* Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Oliver Hartkopp @ 2017-10-19 20:17 UTC (permalink / raw)
To: Mario Hüttel, Marc Kleine-Budde, Sekhar Nori,
Franklin S Cooper Jr, Yang, Wenyou, wg, quentin.schulz, edumazet,
linux-can, netdev, linux-kernel
Cc: Wenyou Yang, Dong Aisheng, Quadros, Roger
In-Reply-To: <4b53c9ff-395f-670f-b55c-38ccfb37ac30@gmx.net>
On 10/19/2017 09:54 PM, Mario Hüttel wrote:
> On 10/19/2017 08:35 PM, Oliver Hartkopp wrote:
>> We already have this 'dsample-point' implemented in the ip tool:
>>
>> $ ip link set vcan0 type can help
>> Usage: ip link set DEVICE type can
>> [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
>> [ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1
>> phase-seg2 PHASE-SEG2 [ sjw SJW ] ]
>>
>> [ dbitrate BITRATE [ dsample-point SAMPLE-POINT] ] | <<-- here!
>> [ dtq TQ dprop-seg PROP_SEG dphase-seg1 PHASE-SEG1
>> dphase-seg2 PHASE-SEG2 [ dsjw SJW ] ]
>>
>> But AFAIK m_can is not using that value in m_can_set_bittiming().
>>
> Actually I need some clarification. The sample point of the can core is
> between the two time segments.
> I always thought that the "sample point" options of the ip tool are used
> in the internal
> calculation of the two timing segments and is therefore no individual value.
You are right.
See picture at http://www.bittiming.can-wiki.info/
Usually you can give the bitrate and the sample point (which is at 75%
aka 0.750 by default) and then the kernel-internal bitrate calculating
algorithm calculates the tq prop-seg phase-seg1 phase-seg2 stuff.
Alternatively you can provide the tq prop-seg phase-seg1 phase-seg2
stuff on your own which is set to the CAN controller registers then.
For that reason my remark "m_can is not using that value" was wrong as
m_can just uses the tq prop-seg phase-seg1 phase-seg2 stuff - either
from the bitrate calculation or provided by the user.
Thanks for the question ;-)
Best,
Oliver
^ permalink raw reply
* Re: [PATCH net-next 3/3] strparser: Generalize strparser
From: John Fastabend @ 2017-10-19 20:18 UTC (permalink / raw)
To: Tom Herbert, Eric Dumazet
Cc: Tom Herbert, Linux Kernel Network Developers, Rohit Seth
In-Reply-To: <CALx6S3765-5oT6UtSQVts4KwWz3F3ub2vhDWO7YemYZPPM0EjA@mail.gmail.com>
On 10/19/2017 12:51 PM, Tom Herbert wrote:
> On Thu, Oct 19, 2017 at 10:42 AM, Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
>
>> On Fri, 2017-07-28 at 16:22 -0700, Tom Herbert wrote:
>>> Generalize strparser from more than just being used in conjunction
>>> with read_sock. strparser will also be used in the send path with
>>> zero proxy. The primary change is to create strp_process function
>>> that performs the critical processing on skbs. The documentation
>>> is also updated to reflect the new uses.
>>>
>>> Signed-off-by: Tom Herbert <tom@quantonium.net>
>>> ---
>>> Documentation/networking/strparser.txt | 207 +++++++++++++++-------
>>> include/net/strparser.h | 119 +++++++------
>>> net/kcm/kcmproc.c | 34 ++--
>>> net/kcm/kcmsock.c | 38 ++--
>>> net/strparser/strparser.c | 313
>> ++++++++++++++++++++-------------
>>> 5 files changed, 424 insertions(+), 287 deletions(-)
>>
>> Just found this gem :
>>
>> static void strp_msg_timeout(unsigned long arg)
>> {
>> struct strparser *strp = (struct strparser *)arg;
>>
>> /* Message assembly timed out */
>> STRP_STATS_INCR(strp->stats.msg_timeouts);
>> strp->cb.lock(strp);
>> strp->cb.abort_parser(strp, ETIMEDOUT);
>> strp->cb.unlock(strp);
>> }
>>
>> static void strp_sock_lock(struct strparser *strp)
>> {
>> lock_sock(strp->sk);
>> }
>>
>> static void strp_sock_unlock(struct strparser *strp)
>> {
>> release_sock(strp->sk);
>> }
>>
>>
>> A timer runs from BH, and from this interrupt context it is absolutely
>> illegal to call lock_sock() ( and release_sock() )
>>
>> Please fix, thanks !
>>
>
> Nice catch! I'll fix it.
>
Can the lock/release be removed and the strp stopped bit can use atomics?
It looks like sk_error_report can be called without sock lock.
Thanks,
John
^ 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