* Re: [PATCH net-next v2 5/5] net: dsa: Stop accessing ds->dst->cpu_dp in drivers
From: Florian Fainelli @ 2017-06-07 19:21 UTC (permalink / raw)
To: Vivien Didelot; +Cc: netdev, Andrew Lunn, John Crispin, David Miller
In-Reply-To: <87ink7oh1h.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On 06/07/2017 10:15 AM, Vivien Didelot wrote:
> Hi Florian,
>
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>>> So as I said in v2, now that a driver is guaranteed that dp->cpu_dp is
>>> correctly assigned at setup time, isn't better (especially for future
>>> multi-CPU support) to provide an helper which returns the CPU port for a
>>> given port? i.e. dsa_get_cpu_port(struct dsa_switch *ds, int port).
>>>
>>> Or is there something blocking? I might be wrong.
>>
>> mt7530.c needs access to the CPU port at ops->setup() time which is
>> why this is still here.
>
> Yes, mt7530 is the only one doing this and has an hardcoded CPU port. So
> what I meant was, shouldn't we have this instead:
>
> struct dsa_port *dsa_get_cpu_port(struct dsa_switch *ds, int port)
> {
> return ds->ports[port].cpu_dp;
> }
We don't actually have a CPU port point to itself:
+
+ for (i = 0; i < ds->num_ports; i++) {
+ p = &ds->ports[i];
+ if (!dsa_port_is_valid(p) ||
+ i == index) <=============
+ continue;
+
+ p->cpu_dp = port;
+ }
}
>
> And:
>
> - dn = ds->dst->cpu_dp->netdev->dev.of_node->parent;
> + cpu_dp = dsa_get_cpu_port(ds, MT7530_CPU_PORT);
> + dn = cpu_dp->netdev->dev.of_node->parent;
If we are giving the port number to get its cpu_dp pointer back, that
seems a bit pointless.
I still think the helper with fls(ds->cpu_port_mask) - 1 is better in
that it will return what you have configured from Device Tree/platform
data. MT7530 does allow the CPU port being arbitrary, and it would
disable MTK tags in that case.
Thanks!
--
Florian
^ permalink raw reply
* Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Jason Gunthorpe @ 2017-06-07 19:21 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Ilan Tayari, Alexei Starovoitov, David S. Miller, Doug Ledford,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
jsorensen@fb.com, Andy Shevchenko, linux-fpga@vger.kernel.org,
Alan Tull, yi1.li@linux.intel.com, Boris Pismenny
In-Reply-To: <CALzJLG8epxWp1pmTd_fxOUfzXF5YoOFfnrRBJ0-YZVL1Gi_yOw@mail.gmail.com>
On Wed, Jun 07, 2017 at 10:13:43PM +0300, Saeed Mahameed wrote:
> On Wed, Jun 7, 2017 at 6:48 PM, Jason Gunthorpe
> <jgunthorpe@obsidianresearch.com> wrote:
> > On Wed, Jun 07, 2017 at 07:16:42AM +0300, Saeed Mahameed wrote:
> >> On Tue, Jun 6, 2017 at 7:17 PM, Jason Gunthorpe
> >> <jgunthorpe@obsidianresearch.com> wrote:
> >> > On Tue, Jun 06, 2017 at 06:52:15AM +0000, Ilan Tayari wrote:
> >> >
> >> >> So neither the host stack nor the network are aware of them.
> >> >> They exist momentarily only on the internal traces on the board and not
> >> >> anywhere else.
> >> >
> >> > Is that really true? If you are creating rocee QPs' then the RDMA
> >> > stack sees this stuff and now we have buried a RDMA ULP inside an
> >> > ethernet driver which seems really wonky..
> >>
> >> It is not an ethernet driver, mlx5_core provides both RDMA and
> >> ethernet interfaces to both mlx5_ib and the mlx5e netdevice.
> >>
> >> so it is perfectly capable of creating QPs on its own, after all it is
> >> the one creating QPs for the RDMA stack :).
> >>
> >> rdma_create_qp->mlx5_ib_create_qp->mlx5_core_create_qp.
> >
> > Wait, so you built a RDMA ULP inside your driver without using the
> > RDMA API?
> >
>
> No !!
> I am just showing you that the ib_core eventually will end up calling
> mlx5_core to create a QP.
> so mlx5_core can create the QP it self since it is the one eventually
> creating QPs.
> we just call mlx5_core_create_qp directly.
Which is building a RDMA ULP inside a driver without using the core
code :(
> > This keep getting more ugly :(
> >
> > What about security? What if user space sends some raw packets to the
> > FPGA - can it reprogram the ISPEC settings or worse?
> >
>
> No such thing. This QP is only for internal driver/HW communications,
> as it is faster from the existing command interface.
> it is not meant to be exposed for any raw user space usages at all,
> without proper standard API adapter of course.
I'm not asking about the QP, I'm asking what happens after the NIC
part. You use ROCE packets to control the FPGA. What prevents
userspace from forcibly constructing roce packets and sending them to
the FPGA. How does the FPGA know for certain the packet came from the
kernel QP and not someplace else.
This is especially true for mlx nics as there are many raw packet
bypass mechanisms available to userspace.
Jason
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: Fix tids count for ipv6 offload connection
From: David Miller @ 2017-06-07 19:15 UTC (permalink / raw)
To: ganeshgr
Cc: netdev, nirranjan, indranil, swise, dledford, linux-rdma, jejb,
linux-scsi, nab, target-devel, rizwana, varun
In-Reply-To: <1496828091-13947-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Wed, 7 Jun 2017 15:04:51 +0530
> the adapter consumes two tids for every ipv6 offload
> connection be it active or passive, calculate tid usage
> count accordingly.
>
> Also change the signatures of relevant functions to get
> the address family.
>
> Signed-off-by: Rizwan Ansari <rizwana@chelsio.com>
> Signed-off-by: Varun Prakash <varun@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied, thanks.
^ permalink raw reply
* Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Saeed Mahameed @ 2017-06-07 19:13 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Ilan Tayari, Alexei Starovoitov, David S. Miller, Doug Ledford,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
jsorensen@fb.com, Andy Shevchenko, linux-fpga@vger.kernel.org,
Alan Tull, yi1.li@linux.intel.com, Boris Pismenny
In-Reply-To: <20170607154858.GA30124@obsidianresearch.com>
On Wed, Jun 7, 2017 at 6:48 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Wed, Jun 07, 2017 at 07:16:42AM +0300, Saeed Mahameed wrote:
>> On Tue, Jun 6, 2017 at 7:17 PM, Jason Gunthorpe
>> <jgunthorpe@obsidianresearch.com> wrote:
>> > On Tue, Jun 06, 2017 at 06:52:15AM +0000, Ilan Tayari wrote:
>> >
>> >> So neither the host stack nor the network are aware of them.
>> >> They exist momentarily only on the internal traces on the board and not
>> >> anywhere else.
>> >
>> > Is that really true? If you are creating rocee QPs' then the RDMA
>> > stack sees this stuff and now we have buried a RDMA ULP inside an
>> > ethernet driver which seems really wonky..
>>
>> It is not an ethernet driver, mlx5_core provides both RDMA and
>> ethernet interfaces to both mlx5_ib and the mlx5e netdevice.
>>
>> so it is perfectly capable of creating QPs on its own, after all it is
>> the one creating QPs for the RDMA stack :).
>>
>> rdma_create_qp->mlx5_ib_create_qp->mlx5_core_create_qp.
>
> Wait, so you built a RDMA ULP inside your driver without using the
> RDMA API?
>
No !!
I am just showing you that the ib_core eventually will end up calling
mlx5_core to create a QP.
so mlx5_core can create the QP it self since it is the one eventually
creating QPs.
we just call mlx5_core_create_qp directly.
> This keep getting more ugly :(
>
> What about security? What if user space sends some raw packets to the
> FPGA - can it reprogram the ISPEC settings or worse?
>
No such thing. This QP is only for internal driver/HW communications,
as it is faster from the existing command interface.
it is not meant to be exposed for any raw user space usages at all,
without proper standard API adapter of course.
> Jason
^ permalink raw reply
* Re: [PATCH 2/2(net.git)] stmmac: fix for hw timestamp of GMAC3 unit
From: David Miller @ 2017-06-07 19:13 UTC (permalink / raw)
To: Mario_Molitor; +Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-kernel
In-Reply-To: <trinity-988824f2-9aed-4ed0-bc97-d78b13337cf5-1496825121649@3capp-webde-bs41>
From: "Mario Molitor" <Mario_Molitor@web.de>
Date: Wed, 7 Jun 2017 10:45:21 +0200
> fixed: commit ba1ffd74df74 ("stmmac: fix PTP support for GMAC4")
The correct tag string is "Fixes: " not "fixed: "
Also, the word "commit" should not be there.
Please fix this in both of your patches.
^ permalink raw reply
* Re: [PATCH 9/9] arm64: marvell: dts: add xmdio nodes for 7k/8k
From: David Miller @ 2017-06-07 19:09 UTC (permalink / raw)
To: gregory.clement
Cc: antoine.tenart, jason, andrew, sebastian.hesselbarth, f.fainelli,
thomas.petazzoni, mw, linux, netdev, linux-arm-kernel
In-Reply-To: <87k24onq62.fsf@free-electrons.com>
From: Gregory CLEMENT <gregory.clement@free-electrons.com>
Date: Wed, 07 Jun 2017 10:43:49 +0200
> If you agrees with this series please don't apply this patch. I will
> take care of it. We have many changes in the dt directory for the next
> release and I want that all the change are in the same place to take
> care of the merge conflict.
Ok, I will elide this patch when/if I apply the series to net-next.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] net: mvpp2: do not bypass the mvpp22_port_mii_set function
From: David Miller @ 2017-06-07 19:08 UTC (permalink / raw)
To: antoine.tenart
Cc: netdev, thomas.petazzoni, gregory.clement, mw, linux,
linux-arm-kernel
In-Reply-To: <20170607061750.28516-1-antoine.tenart@free-electrons.com>
From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Wed, 7 Jun 2017 08:17:50 +0200
> The mvpp22_port_mii_set() function was added by 2697582144dd, but the
> function directly returns without doing anything. This return was used
> when debugging and wasn't removed before sending the patch. Fix this.
>
> Fixes: 2697582144dd ("net: mvpp2: handle misc PPv2.1/PPv2.2 differences")
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net] ibmvnic: Return failure on attempted mtu change
From: David Miller @ 2017-06-07 19:03 UTC (permalink / raw)
To: jallen; +Cc: netdev, tlfalcon, nfont
In-Reply-To: <103eefae-37aa-a30a-4027-5a9c8def3d3f@linux.vnet.ibm.com>
From: John Allen <jallen@linux.vnet.ibm.com>
Date: Tue, 6 Jun 2017 16:55:52 -0500
> Changing the mtu is currently not supported in the ibmvnic driver.
>
> Implement .ndo_change_mtu in the driver so that attempting to use ifconfig
> to change the mtu will fail and present the user with an error message.
>
> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 3/5] net: dsa: add CPU and DSA ports as VLAN members
From: David Miller @ 2017-06-07 19:00 UTC (permalink / raw)
To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew
In-Reply-To: <20170606205631.22880-4-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Tue, 6 Jun 2017 16:56:29 -0400
> @@ -166,6 +166,9 @@ static int dsa_switch_vlan_add(struct dsa_switch *ds,
> bitmap_zero(members, ds->num_ports);
> if (ds->index == info->sw_index)
> set_bit(info->port, members);
> + for (port = 0; port < ds->num_ports; ++port)
> + if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
> + set_bit(port, members);
Please use the more canonical "x++" post-increment in the for() statement.
Thank you.
^ permalink raw reply
* [PATCH 4/4] net: qrtr: Inform open sockets about new controller
From: Bjorn Andersson @ 2017-06-07 18:55 UTC (permalink / raw)
To: David S. Miller
Cc: Arun Kumar Neelakantam, netdev, linux-arm-msm, linux-kernel,
Courtney Cavin
In-Reply-To: <20170607185501.6000-1-bjorn.andersson@linaro.org>
As the higher level communication only deals with "services" the
a service directory is required to keep track of local and remote
services. In order for qrtr clients to be informed about when the
service directory implementation is available some event needs to be
passed to them.
Rather than introducing support for broadcasting such a message in-band
to all open local sockets we flag each socket with ENETRESET, as there
are no other expected operations that would benefit from having support
from locally broadcasting messages.
Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
net/qrtr/qrtr.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index d7516098b5aa..c7a5d861906b 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -530,6 +530,26 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
return 0;
}
+/* Reset all non-control ports */
+static void qrtr_reset_ports(void)
+{
+ struct qrtr_sock *ipc;
+ int id;
+
+ mutex_lock(&qrtr_port_lock);
+ idr_for_each_entry(&qrtr_ports, ipc, id) {
+ /* Don't reset control port */
+ if (id == 0)
+ continue;
+
+ sock_hold(&ipc->sk);
+ ipc->sk.sk_err = ENETRESET;
+ wake_up_interruptible(sk_sleep(&ipc->sk));
+ sock_put(&ipc->sk);
+ }
+ mutex_unlock(&qrtr_port_lock);
+}
+
/* Bind socket to address.
*
* Socket should be locked upon call.
@@ -558,6 +578,10 @@ static int __qrtr_bind(struct socket *sock,
sock_reset_flag(sk, SOCK_ZAPPED);
+ /* Notify all open ports about the new controller */
+ if (port == QRTR_PORT_CTRL)
+ qrtr_reset_ports();
+
return 0;
}
--
2.12.0
^ permalink raw reply related
* [PATCH 3/4] net: qrtr: Broadcast DEL_CLIENT message when endpoint is closed
From: Bjorn Andersson @ 2017-06-07 18:55 UTC (permalink / raw)
To: David S. Miller
Cc: Arun Kumar Neelakantam, netdev, linux-arm-msm, linux-kernel,
Courtney Cavin
In-Reply-To: <20170607185501.6000-1-bjorn.andersson@linaro.org>
Per the QMUXv2 protocol specificiation a DEL_CLIENT message should be
broadcasted when an endpoint is disconnected.
The protocol specification does suggest that the router can keep track
of which nodes the endpoint has been communicating with to not wake up
sleeping remotes unecessarily, but implementation of this suggestion is
left for the future.
Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
net/qrtr/qrtr.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index e8cbab23d667..d7516098b5aa 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -112,6 +112,7 @@ struct qrtr_node {
};
static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb);
+static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb);
/* Release node resources and free the node.
*
@@ -312,6 +313,26 @@ static struct sk_buff *qrtr_alloc_local_bye(u32 src_node)
return skb;
}
+static struct sk_buff *qrtr_alloc_del_client(struct sockaddr_qrtr *sq)
+{
+ const int pkt_len = 20;
+ struct sk_buff *skb;
+ __le32 *buf;
+
+ skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_DEL_CLIENT, pkt_len,
+ sq->sq_node, QRTR_NODE_BCAST);
+ if (!skb)
+ return NULL;
+
+ buf = (__le32 *)skb_put(skb, pkt_len);
+ memset(buf, 0, pkt_len);
+ buf[0] = cpu_to_le32(QRTR_TYPE_DEL_CLIENT);
+ buf[1] = cpu_to_le32(sq->sq_node);
+ buf[2] = cpu_to_le32(sq->sq_port);
+
+ return skb;
+}
+
static struct qrtr_sock *qrtr_port_lookup(int port);
static void qrtr_port_put(struct qrtr_sock *ipc);
@@ -448,8 +469,15 @@ static void qrtr_port_put(struct qrtr_sock *ipc)
/* Remove port assignment. */
static void qrtr_port_remove(struct qrtr_sock *ipc)
{
+ struct sk_buff *skb;
int port = ipc->us.sq_port;
+ skb = qrtr_alloc_del_client(&ipc->us);
+ if (skb) {
+ skb_set_owner_w(skb, &ipc->sk);
+ qrtr_bcast_enqueue(NULL, skb);
+ }
+
if (port == QRTR_PORT_CTRL)
port = 0;
--
2.12.0
^ permalink raw reply related
* [PATCH 2/4] net: qrtr: Inject BYE on remote termination
From: Bjorn Andersson @ 2017-06-07 18:54 UTC (permalink / raw)
To: David S. Miller
Cc: Arun Kumar Neelakantam, netdev, linux-arm-msm, linux-kernel,
Courtney Cavin
In-Reply-To: <20170607185501.6000-1-bjorn.andersson@linaro.org>
Per the QMUX protocol specification a terminating node can send a BYE
control message to signal that the link is going down, upon receiving
this all information about remote services should be discarded and local
clients should be notified.
In the event that the link was brought down abruptly the router is
supposed to act like a BYE message has arrived. As there is no harm in
receiving an extra BYE from the remote this patch implements the latter
by injecting a BYE when the link to the remote is unregistered.
The name service will receive the BYE and can implement the notification
to the local clients.
Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
net/qrtr/qrtr.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 86d35ed50da9..e8cbab23d667 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -111,6 +111,8 @@ struct qrtr_node {
struct list_head item;
};
+static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb);
+
/* Release node resources and free the node.
*
* Do not call directly, use qrtr_node_release. To be used with
@@ -291,6 +293,25 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
return skb;
}
+/* Allocate and construct a BYE message to signal remote termination */
+static struct sk_buff *qrtr_alloc_local_bye(u32 src_node)
+{
+ const int pkt_len = 20;
+ struct sk_buff *skb;
+ __le32 *buf;
+
+ skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_BYE, pkt_len,
+ src_node, qrtr_local_nid);
+ if (!skb)
+ return NULL;
+
+ buf = (__le32 *)skb_put(skb, pkt_len);
+ memset(buf, 0, pkt_len);
+ buf[0] = cpu_to_le32(QRTR_TYPE_BYE);
+
+ return skb;
+}
+
static struct qrtr_sock *qrtr_port_lookup(int port);
static void qrtr_port_put(struct qrtr_sock *ipc);
@@ -382,11 +403,17 @@ EXPORT_SYMBOL_GPL(qrtr_endpoint_register);
void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)
{
struct qrtr_node *node = ep->node;
+ struct sk_buff *skb;
mutex_lock(&node->ep_lock);
node->ep = NULL;
mutex_unlock(&node->ep_lock);
+ /* Notify the local controller about the event */
+ skb = qrtr_alloc_local_bye(node->nid);
+ if (skb)
+ qrtr_local_enqueue(NULL, skb);
+
qrtr_node_release(node);
ep->node = NULL;
}
--
2.12.0
^ permalink raw reply related
* [PATCH 1/4] net: qrtr: Refactor packet allocation
From: Bjorn Andersson @ 2017-06-07 18:54 UTC (permalink / raw)
To: David S. Miller
Cc: Arun Kumar Neelakantam, netdev, linux-arm-msm, linux-kernel,
Courtney Cavin
Extract the allocation and filling in the control message header fields
to a separate function in order to reuse this in subsequent patches.
Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
net/qrtr/qrtr.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index a9a8c7d5a4a9..86d35ed50da9 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -245,14 +245,11 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
}
EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
-/* Allocate and construct a resume-tx packet. */
-static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
- u32 dst_node, u32 port)
+static struct sk_buff *qrtr_alloc_ctrl_packet(u32 type, size_t pkt_len,
+ u32 src_node, u32 dst_node)
{
- const int pkt_len = 20;
struct qrtr_hdr *hdr;
struct sk_buff *skb;
- __le32 *buf;
skb = alloc_skb(QRTR_HDR_SIZE + pkt_len, GFP_KERNEL);
if (!skb)
@@ -261,7 +258,7 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
hdr = (struct qrtr_hdr *)skb_put(skb, QRTR_HDR_SIZE);
hdr->version = cpu_to_le32(QRTR_PROTO_VER);
- hdr->type = cpu_to_le32(QRTR_TYPE_RESUME_TX);
+ hdr->type = cpu_to_le32(type);
hdr->src_node_id = cpu_to_le32(src_node);
hdr->src_port_id = cpu_to_le32(QRTR_PORT_CTRL);
hdr->confirm_rx = cpu_to_le32(0);
@@ -269,6 +266,22 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
hdr->dst_node_id = cpu_to_le32(dst_node);
hdr->dst_port_id = cpu_to_le32(QRTR_PORT_CTRL);
+ return skb;
+}
+
+/* Allocate and construct a resume-tx packet. */
+static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
+ u32 dst_node, u32 port)
+{
+ const int pkt_len = 20;
+ struct sk_buff *skb;
+ __le32 *buf;
+
+ skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_RESUME_TX, pkt_len,
+ src_node, dst_node);
+ if (!skb)
+ return NULL;
+
buf = (__le32 *)skb_put(skb, pkt_len);
memset(buf, 0, pkt_len);
buf[0] = cpu_to_le32(QRTR_TYPE_RESUME_TX);
--
2.12.0
^ permalink raw reply related
* Re: [PATCH net-next] tcp: add TCPMemoryPressuresChrono counter
From: David Miller @ 2017-06-07 18:54 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1496776767.736.11.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 06 Jun 2017 12:19:27 -0700
> @@ -320,18 +320,37 @@ struct tcp_splice_state {
> * All the __sk_mem_schedule() is of this nature: accounting
> * is strict, actions are advisory and have some latency.
> */
> -int tcp_memory_pressure __read_mostly;
> +unsigned long tcp_memory_pressure __read_mostly;
> EXPORT_SYMBOL(tcp_memory_pressure);
I know it is not strictly related to your change, but if you are
adding new exports in this area let's do it properly.
The existing tcp_memory_pressure export, as well as these two:
...
> EXPORT_SYMBOL(tcp_enter_memory_pressure);
...
> +EXPORT_SYMBOL(tcp_leave_memory_pressure);
ought to be EXPORT_SYMBOL_GPL(). They are exported for the sake of
ipv6 TCP support, and nothing else.
I'll apply this with that fixed.
Thanks Eric.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/4] tcp: ULP infrastructure
From: David Miller @ 2017-06-07 18:48 UTC (permalink / raw)
To: davejwatson
Cc: ilyal, aviadye, borisp, liranl, matanb, netdev, tom, herbert,
linux-crypto, hannes, eric.dumazet, alexei.starovoitov, nmav,
fridolin.pokorny
In-Reply-To: <20170606170020.GA19182@davejwatson-mba.local>
From: Dave Watson <davejwatson@fb.com>
Date: Tue, 6 Jun 2017 10:00:20 -0700
> +EXPORT_SYMBOL(tcp_register_ulp);
EXPORT_SYMBOL_GPL() please.
> +EXPORT_SYMBOL(tcp_unregister_ulp);
Likewise.
^ permalink raw reply
* Re: [PATCH net] netlink: don't send unknown nsid
From: Flavio Leitner @ 2017-06-07 18:40 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev
In-Reply-To: <58321cd0-6c7f-d4d8-628f-2c124d71ef82@6wind.com>
On Mon, Jun 05, 2017 at 10:40:24AM +0200, Nicolas Dichtel wrote:
> > Let me ask this instead: How do you think userspace should behave when
> > netnsid allocation fails?
> >
> There is two ways to assign a nsid:
> - manually with netlink ('ip netns set'). In this case, the error is reported
> to userspace via netlink.
OK.
> - automatically when a x-netns interface is created. The link-nsid is also
> reported to userspace. If the allocation failed, NETNSA_NSID_NOT_ASSIGNED is
> reported. And if you were able to create this x-netns interface, it means
> that you have access to this peer netns, thus you can try to assign the nsid
> manually.
Does that prevent the interface to be created?
> So, in both cases, userland knows that something went wrong.
> Do you have another scenario in mind?
Let's say the app is restarted, or another monitoring app is executed
with enough perms. How will it identify the error condition?
--
Flavio
^ permalink raw reply
* Re: [PATCH v2] net: davicom: dm9000: Avoid spinlock recursion during dm9000_timeout routine
From: David Miller @ 2017-06-07 18:38 UTC (permalink / raw)
To: liu.xiang6; +Cc: netdev, linux-kernel, liuxiang1999
In-Reply-To: <1496758626-2726-1-git-send-email-liu.xiang6@zte.com.cn>
From: Liu Xiang <liu.xiang6@zte.com.cn>
Date: Tue, 6 Jun 2017 22:17:06 +0800
> On the DM9000B, dm9000_phy_write() is called after the main spinlock
> is held, during the dm9000_timeout() routine. Spinlock recursion
> occurs because the main spinlock is requested again in
> dm9000_phy_write(). So spinlock should be avoided in phy operation
> during the dm9000_timeout() routine.
>
> ---
> v2:
> dm9000_phy_write_reg is extracted from dm9000_phy_write, with no lock,
> do the real phy operation.
I told you during your first submission that this is racy.
Conditional locking in multithreaded driver almost never works
properly.
Once we enter the timeout, another asynchronous thread of control (an
interrupt, timer, whatever) can try to do a PHY operation and skip the
locking because the timeout boolean is set.
That's not right.
You will need to find a more correct solution to this locking problem.
Thanks.
^ permalink raw reply
* Re: Netlink messages without NLM_F_REQUEST flag
From: Jason Gunthorpe @ 2017-06-07 18:35 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Kaike Wan, John Fleck, Ira Weiny, Thomas Graf, Doug Ledford,
Jiri Pirko, RDMA mailing list, linux-netdev
In-Reply-To: <20170607181810.GI1127@mtr-leonro.local>
On Wed, Jun 07, 2017 at 09:18:10PM +0300, Leon Romanovsky wrote:
> > AFAIK, that is different, that is acking and retriggering a single shot
> > notification, not completing a kernel initiated handshake.
>
> It is acking that message from user was received by kernel and now
> processing.
But isn't what is cared about here - the SA thing needs to send a
request to user space and collect a reply, it runs the protocol
backwards from normal.
It is not a notification because the request actually needs a reply,
and ACK's don't help because the reply has content.
It does not use the NOTIFICATION/REQUEST/REPLY sequence because it
does not care about reliability of delivering the REQUEST to user
space.
Jason
^ permalink raw reply
* [PATCH net] net: ipv6: Release route when device is unregistering
From: David Ahern @ 2017-06-07 18:26 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
Roopa reported attempts to delete a bond device that is referenced in a
multipath route is hanging:
$ ifdown bond2 # ifupdown2 command that deletes virtual devices
unregister_netdevice: waiting for bond2 to become free. Usage count = 2
Steps to reproduce:
echo 1 > /proc/sys/net/ipv6/conf/all/ignore_routes_with_linkdown
ip link add dev bond12 type bond
ip link add dev bond13 type bond
ip addr add 2001:db8:2::0/64 dev bond12
ip addr add 2001:db8:3::0/64 dev bond13
ip route add 2001:db8:33::0/64 nexthop via 2001:db8:2::2 nexthop via 2001:db8:3::2
ip link del dev bond12
ip link del dev bond13
The root cause is the recent change to keep routes on a linkdown. Update
the check to detect when the device is unregistering and release the
route for that case.
Fixes: a1a22c12060e4 ("net: ipv6: Keep nexthop of multipath route on admin down")
Reported-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/linux/netdevice.h | 5 +++++
net/ipv6/route.c | 1 +
2 files changed, 6 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3f39d27decf4..c06d93cfd719 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4261,6 +4261,11 @@ static inline const char *netdev_name(const struct net_device *dev)
return dev->name;
}
+static inline bool netdev_unregistering(const struct net_device *dev)
+{
+ return dev->reg_state == NETREG_UNREGISTERING;
+}
+
static inline const char *netdev_reg_state(const struct net_device *dev)
{
switch (dev->reg_state) {
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index dc61b0b5e64e..7cebd954d5bb 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2804,6 +2804,7 @@ static int fib6_ifdown(struct rt6_info *rt, void *arg)
if ((rt->dst.dev == dev || !dev) &&
rt != adn->net->ipv6.ip6_null_entry &&
(rt->rt6i_nsiblings == 0 ||
+ (dev && netdev_unregistering(dev)) ||
!rt->rt6i_idev->cnf.ignore_routes_with_linkdown))
return -1;
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* RE: Netlink messages without NLM_F_REQUEST flag
From: Hefty, Sean @ 2017-06-07 18:26 UTC (permalink / raw)
To: Leon Romanovsky, Jason Gunthorpe
Cc: Wan, Kaike, Fleck, John, Weiny, Ira, Thomas Graf, Doug Ledford,
Jiri Pirko, RDMA mailing list, linux-netdev
In-Reply-To: <20170607181810.GI1127-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
> It is acking that message from user was received by kernel and now
> processing. The message from kernel to user are anyway unreliable [1],
> so I don't understand on which handshake you are talking.
>
> [1] "reliable transmissions from kernel to user are impossible in any
> case"
> https://linux.die.net/man/7/netlink
The SA messages are kernel initiated messages that target a user space daemon (e.g. ibacm) for cached PR data. If that fails because the daemon isn't running or the message is lost, the normal retry mechanism in the kernel should kick in and try sending the SA message to the remote (i.e. real) SA.
- Sean
--
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: Netlink messages without NLM_F_REQUEST flag
From: Leon Romanovsky @ 2017-06-07 18:18 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Kaike Wan, John Fleck, Ira Weiny, Thomas Graf, Doug Ledford,
Jiri Pirko, RDMA mailing list, linux-netdev
In-Reply-To: <20170607170702.GB7507-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]
On Wed, Jun 07, 2017 at 11:07:02AM -0600, Jason Gunthorpe wrote:
> On Wed, Jun 07, 2017 at 08:00:37PM +0300, Leon Romanovsky wrote:
> > On Wed, Jun 07, 2017 at 10:47:50AM -0600, Jason Gunthorpe wrote:
> > > On Wed, Jun 07, 2017 at 07:43:44PM +0300, Leon Romanovsky wrote:
> > > > On Wed, Jun 7, 2017 at 7:37 PM, Jason Gunthorpe
> > > > <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> > > > > On Wed, Jun 07, 2017 at 07:19:01PM +0300, Leon Romanovsky wrote:
> > > > >> It makes me wonder if it is expected behavior for
> > > > >> ibnl_rcv_reply_skb() to handle !NLM_F_REQUEST messages and do we
> > > > >> really need it? What are the scenarios? In my use case, which is
> > > > >> for sure different from yours, I'm always setting NLM_F_REQUEST
> > > > >> while communicating with kernel.
> > > > >
> > > > > If I recall the user space SA code issues REQUESTS from the kernel to
> > > > > userspace, so userspace returns with the response format. This is
> > > > > abnormal for netlink hence the special function.
> > > >
> > > > In netlink semantics, kernel side is supposed to send netlink
> > > > notification message and userspace is supposed to send REQUEST.
> > >
> > > That pattern is for async communications, the SA stuff needs a sync
> > > protocol, unfortunately.
> >
> > There is special flag NLM_F_ACK for it and userspace will set
> > NLM_F_REQUEST | NLM_F_ACK once synchronization is needed.
>
> AFAIK, that is different, that is acking and retriggering a single shot
> notification, not completing a kernel initiated handshake.
It is acking that message from user was received by kernel and now
processing. The message from kernel to user are anyway unreliable [1],
so I don't understand on which handshake you are talking.
[1] "reliable transmissions from kernel to user are impossible in any case"
https://linux.die.net/man/7/netlink
>
> Jason
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: Netlink messages without NLM_F_REQUEST flag
From: Leon Romanovsky @ 2017-06-07 18:13 UTC (permalink / raw)
To: Weiny, Ira
Cc: Jason Gunthorpe, Wan, Kaike, Fleck, John, Thomas Graf,
Doug Ledford, Jiri Pirko, RDMA mailing list, linux-netdev
In-Reply-To: <2807E5FD2F6FDA4886F6618EAC48510E67CCE1CD@CRSMSX101.amr.corp.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2063 bytes --]
On Wed, Jun 07, 2017 at 05:10:35PM +0000, Weiny, Ira wrote:
> >
> > On Wed, Jun 07, 2017 at 10:47:50AM -0600, Jason Gunthorpe wrote:
> > > On Wed, Jun 07, 2017 at 07:43:44PM +0300, Leon Romanovsky wrote:
> > > > On Wed, Jun 7, 2017 at 7:37 PM, Jason Gunthorpe
> > > > <jgunthorpe@obsidianresearch.com> wrote:
> > > > > On Wed, Jun 07, 2017 at 07:19:01PM +0300, Leon Romanovsky wrote:
> > > > >> It makes me wonder if it is expected behavior for
> > > > >> ibnl_rcv_reply_skb() to handle !NLM_F_REQUEST messages and do
> > > > >> we really need it? What are the scenarios? In my use case,
> > > > >> which is for sure different from yours, I'm always setting
> > > > >> NLM_F_REQUEST while communicating with kernel.
> > > > >
> > > > > If I recall the user space SA code issues REQUESTS from the
> > > > > kernel to userspace, so userspace returns with the response
> > > > > format. This is abnormal for netlink hence the special function.
> > > >
> > > > In netlink semantics, kernel side is supposed to send netlink
> > > > notification message and userspace is supposed to send REQUEST.
> > >
> > > That pattern is for async communications, the SA stuff needs a sync
> > > protocol, unfortunately.
> >
> > There is special flag NLM_F_ACK for it and userspace will set
> > NLM_F_REQUEST | NLM_F_ACK once synchronization is needed.
> >
>
> Reference?
>
> From my understanding, NLM_F_REQUEST | NLM_F_ACK is simply requesting an ack from the kernel on a request. In our case the message is a response to the kernel request.
There is a large chance that we are talking about different aspects of
"sync protocol" and for sure it is due to my misunderstanding. Can you
elaborate more about the protocol? Why is so unique to RDMA?
Why is ACK + sequence number tracking not enough?
https://www.infradead.org/~tgr/libnl/doc/core.html#core_msg_ack
>
> Ira
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH net] net: Zero ifla_vf_info in rtnl_fill_vfinfo()
From: Yuval Mintz @ 2017-06-07 18:00 UTC (permalink / raw)
To: davem, netdev; +Cc: Yuval Mintz, Michal Schmidt
Some of the structure's fields are not initialized by the
rtnetlink. If driver doesn't set those in ndo_get_vf_config(),
they'd leak memory to user.
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
CC: Michal Schmidt <mschmidt@redhat.com>
---
net/core/rtnetlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 9e2c0a7..5e61456 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1124,6 +1124,8 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
struct ifla_vf_mac vf_mac;
struct ifla_vf_info ivi;
+ memset(&ivi, 0, sizeof(ivi));
+
/* Not all SR-IOV capable drivers support the
* spoofcheck and "RSS query enable" query. Preset to
* -1 so the user space tool can detect that the driver
@@ -1132,7 +1134,6 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
ivi.spoofchk = -1;
ivi.rss_query_en = -1;
ivi.trusted = -1;
- memset(ivi.mac, 0, sizeof(ivi.mac));
/* The default value for VF link state is "auto"
* IFLA_VF_LINK_STATE_AUTO which equals zero
*/
--
2.9.4
^ permalink raw reply related
* Re: [PATCH v3 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: John Garry @ 2017-06-07 17:55 UTC (permalink / raw)
To: Ding Tianhong, leedom, ashok.raj, helgaas, werner, ganeshgr,
asit.k.mallick, patrick.j.cramer, Suravee.Suthikulpanit, Bob.Shaw,
l.stach, amira, gabriele.paoloni, David.Laight, jeffrey.t.kirsher,
catalin.marinas, will.deacon, mark.rutland, robin.murphy, davem,
alexander.duyck, linux-arm-kernel, netdev, linux-pci,
linux-kernel
Cc: Linuxarm
In-Reply-To: <1496826968-10152-3-git-send-email-dingtianhong@huawei.com>
On 07/06/2017 10:16, Ding Tianhong wrote:
Hi Ding,
A few general style comments:
> The PCIe Device Control Register use the bit 4 to indicate that
> whether the device is permitted to enable relaxed ordering or not.
> But relaxed ordering is not safe for some platform which could only
> use strong write ordering, so devices are allowed (but not required)
> to enable relaxed ordering bit by default.
>
> If a PCIe device didn't enable the relaxed ordering attribute default,
> we should not do anything in the PCIe configuration, otherwise we
> should check if any of the devices above us do not support relaxed
> ordering by the PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag, then base on
> the result if we get a return that indicate that the relaxed ordering
> is not supported we should update our device to disable relaxed ordering
> in configuration space. If the device above us doesn't exist or isn't
> the PCIe device, we shouldn't do anything and skip updating relaxed ordering
> because we are probably running in a guest.
A guest machine/environment
>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
> drivers/pci/pci.c | 29 +++++++++++++++++++++++++++++
> drivers/pci/probe.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 2 ++
> 3 files changed, 74 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b01bd5b..3d42b38 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4878,6 +4878,35 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
> EXPORT_SYMBOL(pcie_set_mps);
>
> /**
> + * pcie_clear_relaxed_ordering - clear PCI Express relexed ordering bit
> + * @dev: PCI device to query
> + *
> + * If possible clear relaxed ordering
> + */
> +int pcie_clear_relaxed_ordering(struct pci_dev *dev)
> +{
> + return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_RELAX_EN);
> +}
> +EXPORT_SYMBOL(pcie_clear_relaxed_ordering);
> +
> +/**
> + * pcie_get_relaxed_ordering - check PCI Express relexed ordering bit
s/relexed/relaxed/
Check what on relaxed ordering bit?
And the function name is inconsistent with this discription.
> + * @dev: PCI device to query
> + *
> + * Returns true if relaxed ordering is been set
If you want to return true/false, then use !!, below in the function
> + */
> +int pcie_get_relaxed_ordering(struct pci_dev *dev)
> +{
> + u16 v;
> +
> + pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
> +
> + return (v & PCI_EXP_DEVCTL_RELAX_EN) >> 4;
> +}
> +EXPORT_SYMBOL(pcie_get_relaxed_ordering);
> +
> +/**
> * pcie_get_minimum_link - determine minimum link settings of a PCI device
> * @dev: PCI device to query
> * @speed: storage for minimum speed
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..0c94c80 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1701,6 +1701,48 @@ static void pci_configure_extended_tags(struct pci_dev *dev)
> PCI_EXP_DEVCTL_EXT_TAG);
> }
>
> +/**
> + * pci_dev_disable_relaxed_ordering - check if the PCI device
> + * should disable the relaxed ordering attribute.
I think that we need a more accurate description. I know some people
think a function which just "checks" is vague.
> + * @dev: PCI device
> + *
> + * Return true if any of the PCI devices above us do not support
> + * relaxed ordering.
> + */
> +static int pci_dev_disable_relaxed_ordering(struct pci_dev *dev)
The function name implies an action - disabling - but this function does
nothing except return a value
> +{
> + int ro_disabled = 0;
> +
> + while(dev) {
Did you run checkpatch?
> + if (dev->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING) {
> + ro_disabled = 1;
just return true, and return false at the bottom, so you can do away
with ro_disabled (which is not a bool)
> + break;
> + }
> + dev = dev->bus->self;
> + }
> +
> + return ro_disabled;
> +}
> +
> +static void pci_configure_relaxed_ordering(struct pci_dev *dev)
> +{
> + struct pci_dev *bridge = pci_upstream_bridge(dev);
> + int origin_ero;
You don't need this variable
> +
> + if (!pci_is_pcie(dev) || !bridge || !pci_is_pcie(bridge))
> + return;
> +
> + origin_ero = pcie_get_relaxed_ordering(dev);
> + /* If the releaxed ordering enable bit is not set, do nothing. */
> + if (!origin_ero)
> + return;
> +
> + if (pci_dev_disable_relaxed_ordering(dev)) {
> + pcie_clear_relaxed_ordering(dev);
> + dev_info(&dev->dev, "Disable Relaxed Ordering\n");
> + }
> +}
> +
> static void pci_configure_device(struct pci_dev *dev)
> {
> struct hotplug_params hpp;
> @@ -1708,6 +1750,7 @@ static void pci_configure_device(struct pci_dev *dev)
>
> pci_configure_mps(dev);
> pci_configure_extended_tags(dev);
> + pci_configure_relaxed_ordering(dev);
>
> memset(&hpp, 0, sizeof(hpp));
> ret = pci_get_hp_params(dev, &hpp);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e1e8428..299d2f3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1105,6 +1105,8 @@ int __pci_enable_wake(struct pci_dev *dev, pci_power_t state,
> void pci_pme_wakeup_bus(struct pci_bus *bus);
> void pci_d3cold_enable(struct pci_dev *dev);
> void pci_d3cold_disable(struct pci_dev *dev);
> +int pcie_clear_relaxed_ordering(struct pci_dev *dev);
> +int pcie_get_relaxed_ordering(struct pci_dev *dev);
>
> static inline int pci_enable_wake(struct pci_dev *dev, pci_power_t state,
> bool enable)
>
^ permalink raw reply
* RE: [PATCH net-next 00/16] nfp: ctrl vNIC
From: Mintz, Yuval @ 2017-06-07 17:48 UTC (permalink / raw)
To: Jiri Pirko
Cc: Jakub Kicinski, netdev@vger.kernel.org, oss-drivers@netronome.com
In-Reply-To: <20170606112043.GD1911@nanopsycho>
> >> >> >What were your plans with pre-netdev config?
> >> >>
> >> >> We need to pass come initial resource division. Generally the
> >> >> consensus is to have these options exposed through devlink, let
> >> >> the user configure them all and then to have a trigger that would
> >> >> cause driver re-orchestration according to the new values. The
> >> >> flow would look like
> >> >> this:
> >> >>
> >> >> -driver loads with defaults, inits hw and instantiates netdevs
> >> >> -driver exposes config options via devlink -user sets up the
> >> >> options -user pushes the "go" trigger -upon the trigger command,
> >> >> devlink calls the driver re-init callback -driver shuts down the
> >> >> current instances, re-initializes hw, re-instantiates the netdevs
> >> >>
> >> >> Makes sense?
> >> >
> >> >I like the idea of a "go"/apply/reload trigger and extending devlink.
> >> >Do you plan on adding a way to persist the settings? I'm concerned
> >> >NIC users may want to boot into the right mode once it's set,
> >> >without reloads and reconfigs upon boot. Also is there going to be
> >> >a way to query the pending/running config? Sounds like we may want
> >> >to expose three value sets - persistent/default, running and
> >> >pending/to be applied.
> >
> >> I don't think it is a good idea to introduce any kind of
> >> configuration persistency in HW. I believe that user is the master
> >> and he has all needed info. He can store it persistently, but it is up to
> him.
> >>
> >> So basicaly during boot, we need the devlink configuration to happen
> >> early on, before the netdevices get configured. udev? Not sure how
> >> exactly to do this. Have to ask around :)
> >
> >Thinking about use cases where we'd want information available at probe
> >time, it might have been even better to have it separated from the
> >netdevice, e.g., providing clients with node to configure (generic?)
> >information on top of their PCI nodes.
>
> Yuval, devlink is separated from the netdevices....
Separate from the netdevices, yes. But I think it's still a networking facility.
I.e., would it make sense creating devlink nodes for PCI devices?
Anyway, I don't think there's any *strong* need for what I was asking for;
It's simply that I was thinking of qed where there's quite a bit going on
during the pci probe, and thought how re-doing it can be avoided.
^ 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