* Re: marvell switch
From: Andrew Lunn @ 2018-04-05 18:04 UTC (permalink / raw)
To: Ran Shalit; +Cc: netdev
In-Reply-To: <CAJ2oMhL5APVabJuk9R6Bp17VpEg0HZqdkA88tNhajZumsB+f6g@mail.gmail.com>
On Thu, Apr 05, 2018 at 05:26:37PM +0000, Ran Shalit wrote:
> בתאריך יום ה׳, 5 באפר׳ 2018, 19:09, מאת Andrew Lunn <andrew@lunn.ch>:
>
> > > Is there a wiki which explains switch configuration ?
> >
> > Nope. The whole idea is that they behave like normal linux
> > interfaces. So there is no need to document them. You already know how
> > to use them.
> >
> > > Is it possible to open socket and send/recieve on switch ports (lan0
> > > for example) ?
> >
> > Sure. It is as normal interface. Give is an IP address and then do
> > TCP/IP as usual.
> >
>
> There is something I don't fully understand.
> I thought that ports in switch which are not connected to the manage cpu
> are only configured by cpu and then can coomunicate with each other.
>
> What does it mean to open socket in such port (not the cpu port) and send
> data (ethernet frames) ? Does it mean that the cpu port is sending data
> directly to that port ?
The CPU port is configured to use {E}DSA tagging. Frames ingressing on
the CPU port contain an extra header. That header tells the switch
which port to send the frame out of. The switch can also send frames
received on a port out the CPU port to the host, again, with a
header. The host can tell from the header which port the frame
ingressed.
Using this, we can make ports look like normal Linux interfaces.
Andrew
^ permalink raw reply
* [PATCH net-next] hv_netvsc: Add NetVSP v6 into version negotiation
From: Haiyang Zhang @ 2018-04-05 18:42 UTC (permalink / raw)
To: davem, netdev; +Cc: olaf, sthemmin, haiyangz, linux-kernel, devel, vkuznets
From: Haiyang Zhang <haiyangz@microsoft.com>
This patch adds the NetVSP v6 message structures, and includes this
version into NetVSC/NetVSP version negotiation process.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 33 +++++++++++++++++++++++++++++++++
drivers/net/hyperv/netvsc.c | 3 ++-
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 960f06141472..036cd55c66fe 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -237,6 +237,7 @@ void netvsc_switch_datapath(struct net_device *nv_dev, bool vf);
#define NVSP_PROTOCOL_VERSION_2 0x30002
#define NVSP_PROTOCOL_VERSION_4 0x40000
#define NVSP_PROTOCOL_VERSION_5 0x50000
+#define NVSP_PROTOCOL_VERSION_6 0x60000
enum {
NVSP_MSG_TYPE_NONE = 0,
@@ -308,6 +309,11 @@ enum {
NVSP_MSG5_TYPE_SEND_INDIRECTION_TABLE,
NVSP_MSG5_MAX = NVSP_MSG5_TYPE_SEND_INDIRECTION_TABLE,
+
+ /* Version 6 messages */
+ NVSP_MSG6_TYPE_PD_API,
+
+ NVSP_MSG6_MAX = NVSP_MSG6_TYPE_PD_API
};
enum {
@@ -619,12 +625,39 @@ union nvsp_5_message_uber {
struct nvsp_5_send_indirect_table send_table;
} __packed;
+enum nvsp6_pd_api_op {
+ PD_API_OP_NOTIFY_VSP = 0,
+ PD_API_OP_CONFIG,
+ PD_API_OP_MAX
+};
+
+struct nvsp_6_pd_api_req {
+ u32 op;
+ u64 mmio_pa; /* MMIO Physical Address */
+ u32 mmio_len;
+ u32 num_subchn; /* Number of PD subchannels */
+} __packed;
+
+struct nvsp_6_pd_api_comp {
+ u32 op;
+ u32 status;
+ u32 num_subchn; /* Number of PD subchannels */
+ u8 is_supported; /* Is supported by VSP */
+ u8 is_enabled; /* Is enabled by VSP */
+} __packed;
+
+union nvsp_6_message_uber {
+ struct nvsp_6_pd_api_req pd_req;
+ struct nvsp_6_pd_api_comp pd_comp;
+} __packed;
+
union nvsp_all_messages {
union nvsp_message_init_uber init_msg;
union nvsp_1_message_uber v1_msg;
union nvsp_2_message_uber v2_msg;
union nvsp_4_message_uber v4_msg;
union nvsp_5_message_uber v5_msg;
+ union nvsp_6_message_uber v6_msg;
} __packed;
/* ALL Messages */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index c9910c33e671..3abe57bd85bb 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -509,7 +509,8 @@ static int netvsc_connect_vsp(struct hv_device *device,
struct net_device *ndev = hv_get_drvdata(device);
static const u32 ver_list[] = {
NVSP_PROTOCOL_VERSION_1, NVSP_PROTOCOL_VERSION_2,
- NVSP_PROTOCOL_VERSION_4, NVSP_PROTOCOL_VERSION_5
+ NVSP_PROTOCOL_VERSION_4, NVSP_PROTOCOL_VERSION_5,
+ NVSP_PROTOCOL_VERSION_6
};
struct nvsp_message *init_packet;
int ndis_version, i, ret;
--
2.15.1
^ permalink raw reply related
* Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh
From: Jiri Olsa @ 2018-04-05 18:59 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, lkml, netdev,
Linux Kbuild mailing list, Quentin Monnet, Eugene Syromiatnikov,
Jiri Benc, Stanislav Kozina, Jerome Marchand,
Arnaldo Carvalho de Melo, Michal Marek, Jiri Kosina
In-Reply-To: <CAK7LNAQHEWAk_J8uZ4_ETR=JzBU9szDCVNxmUvY447uMj1XLVA@mail.gmail.com>
On Fri, Apr 06, 2018 at 12:50:00AM +0900, Masahiro Yamada wrote:
> 2018-04-06 0:16 GMT+09:00 Jiri Olsa <jolsa@kernel.org>:
> > There's no need to pass LD* arguments to link-vmlinux.sh,
> > because they are passed as variables. The only argument
> > the link-vmlinux.sh supports is the 'clean' argument.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
>
> Wrong.
>
> $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
> exist here so that any change in them
> invokes scripts/linkk-vmlinux.sh
sry, I can't see that.. but it's just a side fix,
which is actually not needed for the rest
I'll check on more and address this separately
thanks,
jirka
>
>
>
>
> > Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index d3300e46f925..a65a3919c6ad 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
> >
> > # Final link of vmlinux with optional arch pass after final link
> > cmd_link-vmlinux = \
> > - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ; \
> > + $(CONFIG_SHELL) $< ; \
> > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
> >
> > vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
>
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 07/12] brcmfmac: Convert ALLFFMAC to ether_broadcast_addr
From: Arend van Spriel @ 2018-04-05 19:00 UTC (permalink / raw)
To: Joe Perches, linux-kernel
Cc: Franky Lin, Hante Meuleman, Chi-Hsien Lin, Wright Feng,
Kalle Valo, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list, netdev
In-Reply-To: <f60d14eaf9027774cff547b82cfb01d0904f6391.1522479608.git.joe@perches.com>
On 3/31/2018 9:05 AM, Joe Perches wrote:
> Remove the local ALLFFMAC extern array and use the new global instead.
I stumbled upon this one couple of weeks ago. I moved the definition to
flowring.c although I considered for a moment to pick up the task you
took here valiantly.
> Miscellanea:
>
> o Convert char *mac to const char *mac as it can't be modified
The real reason is off course that your new global is const and thus mac
variable need to be const as well to avoid compiler warning.
I have to agree with Kalle regarding the upstream logistics, but for
what it is worth...
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 --
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 --
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 8 ++++----
> 3 files changed, 4 insertions(+), 8 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] dt: bindings: add new dt entries for brcmfmac
From: Ulf Hansson @ 2018-04-05 19:09 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Florian Fainelli, Alexey Roslyakov, Andrew Lunn,
Rob Herring, Mark Rutland, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, netdev, linux-wireless, devicetree,
Linux Kernel Mailing List, brcm80211-dev-list.pdl,
brcm80211-dev-list
In-Reply-To: <87lge1er31.fsf@kamboji.qca.qualcomm.com>
On 5 April 2018 at 15:10, Kalle Valo <kvalo@codeaurora.org> wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> On 20 March 2018 at 10:55, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>
>>>>>> If I get it right, you mean something like this:
>>>>>>
>>>>>> mmc3: mmc@1c12000 {
>>>>>> ...
>>>>>> broken-sg-support;
>>>>>> sd-head-align = 4;
>>>>>> sd-sgentry-align = 512;
>>>>>>
>>>>>> brcmf: wifi@1 {
>>>>>> ...
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> Where dt: bindings documentation for these entries should reside?
>>>>>> In generic MMC bindings? Well, this is the very special case and
>>>>>> mmc-linux maintainer will unlikely to accept these changes.
>>>>>> Also, extra kernel code modification might be required. It could make
>>>>>> quite trivial change much more complex.
>>>>>
>>>>> If the MMC maintainers are not copied on this patch series, it will
>>>>> likely be hard for them to identify this patch series and chime in...
>>>>
>>>> The main question is whether this is indeed a "very special case" as
>>>> Alexey claims it to be or that it is likely to be applicable to other
>>>> device and host combinations as you are suggesting.
>>>>
>>>> If these properties are imposed by the host or host controller it
>>>> would make sense to have these in the mmc bindings.
>>>
>>> BTW, last year we were discussing something similar (I mean related to
>>> alignment requirements) with ath10k SDIO patches and at the time the
>>> patch submitter was proposing to have a bounce buffer in ath10k to
>>> workaround that. I don't remember the details anymore, they are on the
>>> ath10k mailing list archive if anyone is curious to know, but I would
>>> not be surprised if they are similar as here. So there might be a need
>>> to solve this in a generic way (but not sure of course as I haven't
>>> checked the details).
>>
>> I re-call something about these as well, here are the patches. Perhaps
>> I should pick some of them up...
>>
>> https://patchwork.kernel.org/patch/10123137/
>> https://patchwork.kernel.org/patch/10123139/
>> https://patchwork.kernel.org/patch/10123141/
>> https://patchwork.kernel.org/patch/10123143/
>
> Actually I was talking about a different patch, found it now:
>
> ath10k_sdio: DMA bounce buffers for read write
>
> https://patchwork.kernel.org/patch/9979543/
Ah, yes. This is about buffer alignment, particularly when using DMA.
Normally there should be no constraint on the alignment, if the
mmc/sdio controller driver would implement a fallback mechanism from
DMA to PIO mode, in case the buffer can't be used for DMA.
However, I know about cases where simply PIO doesn't work because of
broken HW and in many cases the mmc drivers don't implement the
fallback to PIO even if they could.
Moreover, it seems reasonable to anyway have a way for mmc driver to
inform upper layers about DMA buffer alignment constraints, as to be
able to use DMA as long as possible.
Thoughts?
Kind regards
Uffe
^ permalink raw reply
* [PATCH 0/4] hv_netvsc: Fix shutdown issues on older Windows hosts
From: Mohammed Gamal @ 2018-04-05 19:09 UTC (permalink / raw)
To: netdev, sthemmin
Cc: otubo, Mohammed Gamal, haiyangz, linux-kernel, devel, vkuznets
Guests running on WS2012 hosts would not shutdown when changing network
interface setting (e.g. Number of channels, MTU ... etc).
This patch series addresses these shutdown issues we enecountered with WS2012
hosts. It's essentialy a rework of the series sent in
https://lkml.org/lkml/2018/1/23/111 on top of latest upstream
Fixes: 0ef58b0a05c1 ("hv_netvsc: change GPAD teardown order on older versions")
Mohammed Gamal (4):
hv_netvsc: Use Windows version instead of NVSP version on GPAD
teardown
hv_netvsc: Split netvsc_revoke_buf() and netvsc_teardown_gpadl()
hv_netvsc: Ensure correct teardown message sequence order
hv_netvsc: Pass net_device parameter to revoke and teardown functions
drivers/net/hyperv/netvsc.c | 60 +++++++++++++++++++++++++++++++++------------
1 file changed, 44 insertions(+), 16 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH 1/4] hv_netvsc: Use Windows version instead of NVSP version on GPAD teardown
From: Mohammed Gamal @ 2018-04-05 19:09 UTC (permalink / raw)
To: netdev, sthemmin
Cc: otubo, Mohammed Gamal, haiyangz, linux-kernel, devel, vkuznets
In-Reply-To: <1522955361-14704-1-git-send-email-mgamal@redhat.com>
When changing network interface settings, Windows guests
older than WS2016 can no longer shutdown. This was addressed
by commit 0ef58b0a05c12 ("hv_netvsc: change GPAD teardown order
on older versions"), however the issue also occurs on WS2012
guests that share NVSP protocol versions with WS2016 guests.
Hence we use Windows version directly to differentiate them.
Fixes: 0ef58b0a05c12 ("hv_netvsc: change GPAD teardown order on older versions")
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
drivers/net/hyperv/netvsc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index c9910c3..d65b7fc 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -590,13 +590,13 @@ void netvsc_device_remove(struct hv_device *device)
netdev_dbg(ndev, "net device safe to remove\n");
/* older versions require that buffer be revoked before close */
- if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_4)
+ if (vmbus_proto_version < VERSION_WIN10)
netvsc_teardown_gpadl(device, net_device);
/* Now, we can close the channel safely */
vmbus_close(device->channel);
- if (net_device->nvsp_version >= NVSP_PROTOCOL_VERSION_4)
+ if (vmbus_proto_version >= VERSION_WIN10)
netvsc_teardown_gpadl(device, net_device);
/* Release all resources */
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/4] hv_netvsc: Split netvsc_revoke_buf() and netvsc_teardown_gpadl()
From: Mohammed Gamal @ 2018-04-05 19:09 UTC (permalink / raw)
To: netdev, sthemmin
Cc: otubo, Mohammed Gamal, haiyangz, linux-kernel, devel, vkuznets
In-Reply-To: <1522955361-14704-1-git-send-email-mgamal@redhat.com>
Split each of the functions into two for each of send/recv buffers.
This will be needed in order to implement a fine-grained messaging
sequence to the host so tht we accommodate the requirements of
different Windows versions
Fixes: 0ef58b0a05c12 ("hv_netvsc: change GPAD teardown order on older versions")
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
drivers/net/hyperv/netvsc.c | 46 +++++++++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index d65b7fc..f4df5de 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -109,11 +109,11 @@ static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
call_rcu(&nvdev->rcu, free_netvsc_device);
}
-static void netvsc_revoke_buf(struct hv_device *device,
- struct netvsc_device *net_device)
+static void netvsc_revoke_recv_buf(struct hv_device *device,
+ struct netvsc_device *net_device)
{
- struct nvsp_message *revoke_packet;
struct net_device *ndev = hv_get_drvdata(device);
+ struct nvsp_message *revoke_packet;
int ret;
/*
@@ -157,6 +157,14 @@ static void netvsc_revoke_buf(struct hv_device *device,
}
net_device->recv_section_cnt = 0;
}
+}
+
+static void netvsc_revoke_send_buf(struct hv_device *device,
+ struct netvsc_device *net_device)
+{
+ struct net_device *ndev = hv_get_drvdata(device);
+ struct nvsp_message *revoke_packet;
+ int ret;
/* Deal with the send buffer we may have setup.
* If we got a send section size, it means we received a
@@ -202,8 +210,8 @@ static void netvsc_revoke_buf(struct hv_device *device,
}
}
-static void netvsc_teardown_gpadl(struct hv_device *device,
- struct netvsc_device *net_device)
+static void netvsc_teardown_recv_gpadl(struct hv_device *device,
+ struct netvsc_device *net_device)
{
struct net_device *ndev = hv_get_drvdata(device);
int ret;
@@ -222,6 +230,13 @@ static void netvsc_teardown_gpadl(struct hv_device *device,
}
net_device->recv_buf_gpadl_handle = 0;
}
+}
+
+static void netvsc_teardown_send_gpadl(struct hv_device *device,
+ struct netvsc_device *net_device)
+{
+ struct net_device *ndev = hv_get_drvdata(device);
+ int ret;
if (net_device->send_buf_gpadl_handle) {
ret = vmbus_teardown_gpadl(device->channel,
@@ -437,8 +452,10 @@ static int netvsc_init_buf(struct hv_device *device,
goto exit;
cleanup:
- netvsc_revoke_buf(device, net_device);
- netvsc_teardown_gpadl(device, net_device);
+ netvsc_revoke_recv_buf(device, net_device);
+ netvsc_revoke_send_buf(device, net_device);
+ netvsc_teardown_recv_gpadl(device, net_device);
+ netvsc_teardown_send_gpadl(device, net_device);
exit:
return ret;
@@ -575,7 +592,8 @@ void netvsc_device_remove(struct hv_device *device)
= rtnl_dereference(net_device_ctx->nvdev);
int i;
- netvsc_revoke_buf(device, net_device);
+ netvsc_revoke_recv_buf(device, net_device);
+ netvsc_revoke_send_buf(device, net_device);
RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
@@ -590,14 +608,18 @@ void netvsc_device_remove(struct hv_device *device)
netdev_dbg(ndev, "net device safe to remove\n");
/* older versions require that buffer be revoked before close */
- if (vmbus_proto_version < VERSION_WIN10)
- netvsc_teardown_gpadl(device, net_device);
+ if (vmbus_proto_version < VERSION_WIN10) {
+ netvsc_teardown_recv_gpadl(device, net_device);
+ netvsc_teardown_send_gpadl(device, net_device);
+ }
/* Now, we can close the channel safely */
vmbus_close(device->channel);
- if (vmbus_proto_version >= VERSION_WIN10)
- netvsc_teardown_gpadl(device, net_device);
+ if (vmbus_proto_version >= VERSION_WIN10) {
+ netvsc_teardown_recv_gpadl(device, net_device);
+ netvsc_teardown_send_gpadl(device, net_device);
+ }
/* Release all resources */
free_netvsc_device_rcu(net_device);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/4] hv_netvsc: Ensure correct teardown message sequence order
From: Mohammed Gamal @ 2018-04-05 19:09 UTC (permalink / raw)
To: netdev, sthemmin
Cc: otubo, Mohammed Gamal, haiyangz, linux-kernel, devel, vkuznets
In-Reply-To: <1522955361-14704-1-git-send-email-mgamal@redhat.com>
Prior to commit 0cf737808ae7 ("hv_netvsc: netvsc_teardown_gpadl() split")
the call sequence in netvsc_device_remove() was as follows (as
implemented in netvsc_destroy_buf()):
1- Send NVSP_MSG1_TYPE_REVOKE_RECV_BUF message
2- Teardown receive buffer GPADL
3- Send NVSP_MSG1_TYPE_REVOKE_SEND_BUF message
4- Teardown send buffer GPADL
5- Close vmbus
This didn't work for WS2016 hosts. Commit 0cf737808ae7
("hv_netvsc: netvsc_teardown_gpadl() split") rearranged the
teardown sequence as follows:
1- Send NVSP_MSG1_TYPE_REVOKE_RECV_BUF message
2- Send NVSP_MSG1_TYPE_REVOKE_SEND_BUF message
3- Close vmbus
4- Teardown receive buffer GPADL
5- Teardown send buffer GPADL
That worked well for WS2016 hosts, but it prevented guests on older hosts from
shutting down after changing network settings. Commit 0ef58b0a05c1
("hv_netvsc: change GPAD teardown order on older versions") ensured the
following message sequence for older hosts
1- Send NVSP_MSG1_TYPE_REVOKE_RECV_BUF message
2- Send NVSP_MSG1_TYPE_REVOKE_SEND_BUF message
3- Teardown receive buffer GPADL
4- Teardown send buffer GPADL
5- Close vmbus
However, with this sequence calling `ip link set eth0 mtu 1000` hangs and the
process becomes uninterruptible. On futher analysis it turns out that on tearing
down the receive buffer GPADL the kernel is waiting indefinitely
in vmbus_teardown_gpadl() for a completion to be signaled.
Here is a snippet of where this occurs:
int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
{
struct vmbus_channel_gpadl_teardown *msg;
struct vmbus_channel_msginfo *info;
unsigned long flags;
int ret;
info = kmalloc(sizeof(*info) +
sizeof(struct vmbus_channel_gpadl_teardown), GFP_KERNEL);
if (!info)
return -ENOMEM;
init_completion(&info->waitevent);
info->waiting_channel = channel;
[....]
ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_gpadl_teardown),
true);
if (ret)
goto post_msg_err;
wait_for_completion(&info->waitevent);
[....]
}
The completion is signaled from vmbus_ongpadl_torndown(), which gets called when
the corresponding message is received from the host, which apparently never happens
in that case.
This patch works around the issue by restoring the first mentioned message sequence
for older hosts
Fixes: 0ef58b0a05c1 ("hv_netvsc: change GPAD teardown order on older versions")
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
drivers/net/hyperv/netvsc.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index f4df5de..df92c2f 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -592,8 +592,17 @@ void netvsc_device_remove(struct hv_device *device)
= rtnl_dereference(net_device_ctx->nvdev);
int i;
+ /*
+ * Revoke receive buffer. If host is pre-Win2016 then tear down
+ * receive buffer GPADL. Do the same for send buffer.
+ */
netvsc_revoke_recv_buf(device, net_device);
+ if (vmbus_proto_version < VERSION_WIN10)
+ netvsc_teardown_recv_gpadl(device, net_device);
+
netvsc_revoke_send_buf(device, net_device);
+ if (vmbus_proto_version < VERSION_WIN10)
+ netvsc_teardown_send_gpadl(device, net_device);
RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
@@ -607,15 +616,13 @@ void netvsc_device_remove(struct hv_device *device)
*/
netdev_dbg(ndev, "net device safe to remove\n");
- /* older versions require that buffer be revoked before close */
- if (vmbus_proto_version < VERSION_WIN10) {
- netvsc_teardown_recv_gpadl(device, net_device);
- netvsc_teardown_send_gpadl(device, net_device);
- }
-
/* Now, we can close the channel safely */
vmbus_close(device->channel);
+ /*
+ * If host is Win2016 or higher then we do the GPADL tear down
+ * here after VMBus is closed.
+ */
if (vmbus_proto_version >= VERSION_WIN10) {
netvsc_teardown_recv_gpadl(device, net_device);
netvsc_teardown_send_gpadl(device, net_device);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 4/4] hv_netvsc: Pass net_device parameter to revoke and teardown functions
From: Mohammed Gamal @ 2018-04-05 19:09 UTC (permalink / raw)
To: netdev, sthemmin
Cc: devel, linux-kernel, kys, haiyangz, vkuznets, otubo,
Mohammed Gamal
In-Reply-To: <1522955361-14704-1-git-send-email-mgamal@redhat.com>
The callers to netvsc_revoke_*_buf() and netvsc_teardown_*_gpadl()
already have their net_device instances. Pass them as a paramaeter to
the function instead of obtaining them from netvsc_device struct
everytime
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
drivers/net/hyperv/netvsc.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index df92c2f..04f611e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -110,9 +110,9 @@ static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
}
static void netvsc_revoke_recv_buf(struct hv_device *device,
- struct netvsc_device *net_device)
+ struct netvsc_device *net_device,
+ struct net_device *ndev)
{
- struct net_device *ndev = hv_get_drvdata(device);
struct nvsp_message *revoke_packet;
int ret;
@@ -160,9 +160,9 @@ static void netvsc_revoke_recv_buf(struct hv_device *device,
}
static void netvsc_revoke_send_buf(struct hv_device *device,
- struct netvsc_device *net_device)
+ struct netvsc_device *net_device,
+ struct net_device *ndev)
{
- struct net_device *ndev = hv_get_drvdata(device);
struct nvsp_message *revoke_packet;
int ret;
@@ -211,9 +211,9 @@ static void netvsc_revoke_send_buf(struct hv_device *device,
}
static void netvsc_teardown_recv_gpadl(struct hv_device *device,
- struct netvsc_device *net_device)
+ struct netvsc_device *net_device,
+ struct net_device *ndev)
{
- struct net_device *ndev = hv_get_drvdata(device);
int ret;
if (net_device->recv_buf_gpadl_handle) {
@@ -233,9 +233,9 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device,
}
static void netvsc_teardown_send_gpadl(struct hv_device *device,
- struct netvsc_device *net_device)
+ struct netvsc_device *net_device,
+ struct net_device *ndev)
{
- struct net_device *ndev = hv_get_drvdata(device);
int ret;
if (net_device->send_buf_gpadl_handle) {
@@ -452,10 +452,10 @@ static int netvsc_init_buf(struct hv_device *device,
goto exit;
cleanup:
- netvsc_revoke_recv_buf(device, net_device);
- netvsc_revoke_send_buf(device, net_device);
- netvsc_teardown_recv_gpadl(device, net_device);
- netvsc_teardown_send_gpadl(device, net_device);
+ netvsc_revoke_recv_buf(device, net_device, ndev);
+ netvsc_revoke_send_buf(device, net_device, ndev);
+ netvsc_teardown_recv_gpadl(device, net_device, ndev);
+ netvsc_teardown_send_gpadl(device, net_device, ndev);
exit:
return ret;
@@ -474,7 +474,6 @@ static int negotiate_nvsp_ver(struct hv_device *device,
init_packet->hdr.msg_type = NVSP_MSG_TYPE_INIT;
init_packet->msg.init_msg.init.min_protocol_ver = nvsp_ver;
init_packet->msg.init_msg.init.max_protocol_ver = nvsp_ver;
-
trace_nvsp_send(ndev, init_packet);
/* Send the init request */
@@ -596,13 +595,13 @@ void netvsc_device_remove(struct hv_device *device)
* Revoke receive buffer. If host is pre-Win2016 then tear down
* receive buffer GPADL. Do the same for send buffer.
*/
- netvsc_revoke_recv_buf(device, net_device);
+ netvsc_revoke_recv_buf(device, net_device, ndev);
if (vmbus_proto_version < VERSION_WIN10)
- netvsc_teardown_recv_gpadl(device, net_device);
+ netvsc_teardown_recv_gpadl(device, net_device, ndev);
- netvsc_revoke_send_buf(device, net_device);
+ netvsc_revoke_send_buf(device, net_device, ndev);
if (vmbus_proto_version < VERSION_WIN10)
- netvsc_teardown_send_gpadl(device, net_device);
+ netvsc_teardown_send_gpadl(device, net_device, ndev);
RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
@@ -624,8 +623,8 @@ void netvsc_device_remove(struct hv_device *device)
* here after VMBus is closed.
*/
if (vmbus_proto_version >= VERSION_WIN10) {
- netvsc_teardown_recv_gpadl(device, net_device);
- netvsc_teardown_send_gpadl(device, net_device);
+ netvsc_teardown_recv_gpadl(device, net_device, ndev);
+ netvsc_teardown_send_gpadl(device, net_device, ndev);
}
/* Release all resources */
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next v2 2/2] dt: bindings: add new dt entries for brcmfmac
From: Arend van Spriel @ 2018-04-05 19:11 UTC (permalink / raw)
To: Kalle Valo, Ulf Hansson
Cc: Florian Fainelli, Alexey Roslyakov, Andrew Lunn, Rob Herring,
Mark Rutland, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, netdev, linux-wireless, devicetree,
Linux Kernel Mailing List, brcm80211-dev-list.pdl,
brcm80211-dev-list
In-Reply-To: <87lge1er31.fsf@kamboji.qca.qualcomm.com>
On 4/5/2018 3:10 PM, Kalle Valo wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> On 20 March 2018 at 10:55, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>
>>>>>> If I get it right, you mean something like this:
>>>>>>
>>>>>> mmc3: mmc@1c12000 {
>>>>>> ...
>>>>>> broken-sg-support;
>>>>>> sd-head-align = 4;
>>>>>> sd-sgentry-align = 512;
>>>>>>
>>>>>> brcmf: wifi@1 {
>>>>>> ...
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> Where dt: bindings documentation for these entries should reside?
>>>>>> In generic MMC bindings? Well, this is the very special case and
>>>>>> mmc-linux maintainer will unlikely to accept these changes.
>>>>>> Also, extra kernel code modification might be required. It could make
>>>>>> quite trivial change much more complex.
>>>>>
>>>>> If the MMC maintainers are not copied on this patch series, it will
>>>>> likely be hard for them to identify this patch series and chime in...
>>>>
>>>> The main question is whether this is indeed a "very special case" as
>>>> Alexey claims it to be or that it is likely to be applicable to other
>>>> device and host combinations as you are suggesting.
>>>>
>>>> If these properties are imposed by the host or host controller it
>>>> would make sense to have these in the mmc bindings.
>>>
>>> BTW, last year we were discussing something similar (I mean related to
>>> alignment requirements) with ath10k SDIO patches and at the time the
>>> patch submitter was proposing to have a bounce buffer in ath10k to
>>> workaround that. I don't remember the details anymore, they are on the
>>> ath10k mailing list archive if anyone is curious to know, but I would
>>> not be surprised if they are similar as here. So there might be a need
>>> to solve this in a generic way (but not sure of course as I haven't
>>> checked the details).
>>
>> I re-call something about these as well, here are the patches. Perhaps
>> I should pick some of them up...
>>
>> https://patchwork.kernel.org/patch/10123137/
>> https://patchwork.kernel.org/patch/10123139/
>> https://patchwork.kernel.org/patch/10123141/
>> https://patchwork.kernel.org/patch/10123143/
>
> Actually I was talking about a different patch, found it now:
>
> ath10k_sdio: DMA bounce buffers for read write
>
> https://patchwork.kernel.org/patch/9979543/
The patches Uffe mentions are related to this. In brcmfmac we simply
implemented functionality to create a scatter list and send it through
the MMC stack using SDIO CMD53. It is in the creation of the scatter
list that these alignment properties are needed. Moving the whole
functionality to the MMC stack will also move those properties into
their right context, ie. SDIO host controller.
Our broker_sg_support property is basically doing the bounce buffer
thing if I understand it correctly.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH net 0/6] net: better validate user provided tunnel names
From: David Miller @ 2018-04-05 19:21 UTC (permalink / raw)
To: edumazet; +Cc: netdev, steffen.klassert, eric.dumazet
In-Reply-To: <20180405133931.207634-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 5 Apr 2018 06:39:25 -0700
> This series changes dev_valid_name() to not attempt reading
> a possibly too long user-provided device name, then use
> this helper in five different tunnel providers.
Series applied and queued up for -stable, thanks Eric.
Reading over this series makes me wonder if we generally have an
off-by-one bug for device names which are exactly IFNAMSIZ.
We validate the size using the test:
if (strlen(name) >= IFNAMSIZ)
return ERROR;
and thusly after Eric's changes:
if (strnlen(name, IFNAMSIZ) == IFNAMSIZ)
return ERROR;
This value computed by str{,n}len() doesn't include the trailing null
byte.
So we will accept a name that has exactly IFNAMSIZ bytes long not
including the trailing null.
Then we will copy IFNAMSIZ bytes, minus 1, into the device name and
then tack on the trailling null byte.
So essentially we will set the final non-null byte in the string to
a null byte.
Am I misreading things?
^ permalink raw reply
* Re: [PATCH net 0/6] net: better validate user provided tunnel names
From: Eric Dumazet @ 2018-04-05 19:40 UTC (permalink / raw)
To: David Miller, edumazet; +Cc: netdev, steffen.klassert, eric.dumazet
In-Reply-To: <20180405.152111.2211916878014180558.davem@davemloft.net>
On 04/05/2018 12:21 PM, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Thu, 5 Apr 2018 06:39:25 -0700
>
>> This series changes dev_valid_name() to not attempt reading
>> a possibly too long user-provided device name, then use
>> this helper in five different tunnel providers.
>
> Series applied and queued up for -stable, thanks Eric.
>
> Reading over this series makes me wonder if we generally have an
> off-by-one bug for device names which are exactly IFNAMSIZ.
>
> We validate the size using the test:
>
> if (strlen(name) >= IFNAMSIZ)
> return ERROR;
>
> and thusly after Eric's changes:
>
> if (strnlen(name, IFNAMSIZ) == IFNAMSIZ)
> return ERROR;
>
> This value computed by str{,n}len() doesn't include the trailing null
> byte.
>
> So we will accept a name that has exactly IFNAMSIZ bytes long not
> including the trailing null.
In this case strnlen(name, IFNAMSIZ) returns IFNAMSIZ.
So (strnlen(name, IFNAMSIZ) == IFNAMSIZ) would definitely be true.
The only effect of the change is that strlen() would read 1000 bytes of a
malicious string before we reached the test on the length to reject such name.
While strnlen() is guaranteed to not read more than IFNAMSIZ bytes.
^ permalink raw reply
* Re: [PATCH iproute2] l2tp: no need to export session offsets in JSON output
From: Stephen Hemminger @ 2018-04-05 19:44 UTC (permalink / raw)
To: Guillaume Nault; +Cc: netdev, James Chapman
In-Reply-To: <6e2c59a6757414fa22b5927f248749efe34825d5.1522948642.git.g.nault@alphalink.fr>
On Thu, 5 Apr 2018 19:24:17 +0200
Guillaume Nault <g.nault@alphalink.fr> wrote:
> The offset and peer_offset parameters are only printed to avoid
> confusing external scripts that may parse "ip l2tp show session"
> output. There's no reason to keep them in JSON.
>
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> ---
Applied thanks.
^ permalink raw reply
* Re: [PATCH net-next 6/6] netdevsim: Add simple FIB resource controller via devlink
From: David Ahern @ 2018-04-05 20:10 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski,
andy.roulin
In-Reply-To: <20180405172718.GA9125@nanopsycho>
On 4/5/18 11:27 AM, Jiri Pirko wrote:
> Wed, Mar 28, 2018 at 03:22:00AM CEST, dsa@cumulusnetworks.com wrote:
>> Add devlink support to netdevsim and use it to implement a simple,
>> profile based resource controller. Only one controller is needed
>> per namespace, so the first netdevsim netdevice in a namespace
>> registers with devlink. If that device is deleted, the resource
>> settings are deleted.
>
> I don't understand why you add 1:1 fixed relationship between
> netnamespace and devlink instance. That is highly misleading and reader
> might think that those 2 are somehow related. They are not. You can have
> multiple devlink instances for many ports in a single namespace.
The netdevsim devlink instance is an example of limiting the number of
FIB entries and FIB rules for a namespace. It is currently limited to
the init_net based on past discussion.
It does not make sense to have multiple resource controllers for the
same network namespace, hence the limit of only registering with devlink
on the first device create.
>
> Could you please clarify?
>
> Also, to see the relationship between individual netdevsim netdevices
> and the parent devlink instance, we should use devlink_port
> instances, like this:
>
> devlink1 devlink2
> | | | |
> dl_port1_1 dlport1_2 dlport2_1 dlport2_2
> | | | |
> eth0 eth1 eth2 eth3
>
> Note that "devlink instance" reprensents one ASIC.
> The address of the devlink instance is the bus address of the ASIC.
> Here, you use address of some/first netdevsim netdev instance.
The ASIC here is the kernel tables in a namespace. It does not make
sense to have 2 devlink instances for a single namespace.
>
> The way it is implemented in netdevsim by this patch is wrong on
> so many levels :(
>
> Could you please fix this? I'm more than happy to help you with this,
> please say so. Thanks!
What is there to fix?
Not creating a netdevsim device per netdevsim netdevice? That is
completely unrelated to the devlink change.
^ permalink raw reply
* Enable and configure storm prevention in a network device
From: Murali Karicheri @ 2018-04-05 20:14 UTC (permalink / raw)
To: open list:TI NETCP ETHERNET DRIVER
Hello Netdev experts,
Is there a standard way to implement and configure storm prevention in a Linux
network device?
Our NIC firmware has the capability to enable storm prevention which is implemented
using a credit based scheme. The configuration is how many number of multicast +
broadcast packets allowed in a certain period of time. Is there a standard way
of passing this information from user space to driver?
Thanks in advance for your input!
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* [patch net] devlink: convert occ_get op to separate registration
From: Jiri Pirko @ 2018-04-05 20:13 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, jakub.kicinski, dsahern, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
This resolves race during initialization where the resources with
ops are registered before driver and the structures used by occ_get
op is initialized. So keep occ_get callbacks registered only when
all structs are initialized.
The example flows, as it is in mlxsw:
1) driver load/asic probe:
mlxsw_core
-> mlxsw_sp_resources_register
-> mlxsw_sp_kvdl_resources_register
-> devlink_resource_register IDX
mlxsw_spectrum
-> mlxsw_sp_kvdl_init
-> mlxsw_sp_kvdl_parts_init
-> mlxsw_sp_kvdl_part_init
-> devlink_resource_size_get IDX (to get the current setup
size from devlink)
-> devlink_resource_occ_get_register IDX (register current
occupancy getter)
2) reload triggered by devlink command:
-> mlxsw_devlink_core_bus_device_reload
-> mlxsw_sp_fini
-> mlxsw_sp_kvdl_fini
-> devlink_resource_occ_get_unregister IDX
(struct mlxsw_sp *mlxsw_sp is freed at this point, call to occ get
which is using mlxsw_sp would cause use-after free)
-> mlxsw_sp_init
-> mlxsw_sp_kvdl_init
-> mlxsw_sp_kvdl_parts_init
-> mlxsw_sp_kvdl_part_init
-> devlink_resource_size_get IDX (to get the current setup
size from devlink)
-> devlink_resource_occ_get_register IDX (register current
occupancy getter)
Fixes: d9f9b9a4d05f ("devlink: Add support for resource abstraction")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v1->v2:
- rebased on top or netdevsim changes
- improved description
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 24 ++-----
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 1 -
.../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c | 67 ++++++++++++--------
drivers/net/netdevsim/devlink.c | 65 +++++++++----------
include/net/devlink.h | 40 ++++++++----
net/core/devlink.c | 74 +++++++++++++++++++---
6 files changed, 165 insertions(+), 106 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 53fffd09d133..ca38a30fbe91 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3805,18 +3805,6 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
},
};
-static u64 mlxsw_sp_resource_kvd_linear_occ_get(struct devlink *devlink)
-{
- struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
-
- return mlxsw_sp_kvdl_occ_get(mlxsw_sp);
-}
-
-static const struct devlink_resource_ops mlxsw_sp_resource_kvd_linear_ops = {
- .occ_get = mlxsw_sp_resource_kvd_linear_occ_get,
-};
-
static void
mlxsw_sp_resource_size_params_prepare(struct mlxsw_core *mlxsw_core,
struct devlink_resource_size_params *kvd_size_params,
@@ -3877,8 +3865,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
err = devlink_resource_register(devlink, MLXSW_SP_RESOURCE_NAME_KVD,
kvd_size, MLXSW_SP_RESOURCE_KVD,
DEVLINK_RESOURCE_ID_PARENT_TOP,
- &kvd_size_params,
- NULL);
+ &kvd_size_params);
if (err)
return err;
@@ -3887,8 +3874,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
linear_size,
MLXSW_SP_RESOURCE_KVD_LINEAR,
MLXSW_SP_RESOURCE_KVD,
- &linear_size_params,
- &mlxsw_sp_resource_kvd_linear_ops);
+ &linear_size_params);
if (err)
return err;
@@ -3905,8 +3891,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
double_size,
MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE,
MLXSW_SP_RESOURCE_KVD,
- &hash_double_size_params,
- NULL);
+ &hash_double_size_params);
if (err)
return err;
@@ -3915,8 +3900,7 @@ static int mlxsw_sp_resources_register(struct mlxsw_core *mlxsw_core)
single_size,
MLXSW_SP_RESOURCE_KVD_HASH_SINGLE,
MLXSW_SP_RESOURCE_KVD,
- &hash_single_size_params,
- NULL);
+ &hash_single_size_params);
if (err)
return err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 82820ba43728..804d4d2c8031 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -442,7 +442,6 @@ void mlxsw_sp_kvdl_free(struct mlxsw_sp *mlxsw_sp, int entry_index);
int mlxsw_sp_kvdl_alloc_size_query(struct mlxsw_sp *mlxsw_sp,
unsigned int entry_count,
unsigned int *p_alloc_size);
-u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp);
int mlxsw_sp_kvdl_resources_register(struct mlxsw_core *mlxsw_core);
struct mlxsw_sp_acl_rule_info {
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
index 8796db44dcc3..fe4327f547d2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -315,8 +315,9 @@ static u64 mlxsw_sp_kvdl_part_occ(struct mlxsw_sp_kvdl_part *part)
return occ;
}
-u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
+static u64 mlxsw_sp_kvdl_occ_get(void *priv)
{
+ const struct mlxsw_sp *mlxsw_sp = priv;
u64 occ = 0;
int i;
@@ -326,48 +327,33 @@ u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
return occ;
}
-static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
+static u64 mlxsw_sp_kvdl_single_occ_get(void *priv)
{
- struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+ const struct mlxsw_sp *mlxsw_sp = priv;
struct mlxsw_sp_kvdl_part *part;
part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
return mlxsw_sp_kvdl_part_occ(part);
}
-static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
+static u64 mlxsw_sp_kvdl_chunks_occ_get(void *priv)
{
- struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+ const struct mlxsw_sp *mlxsw_sp = priv;
struct mlxsw_sp_kvdl_part *part;
part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
return mlxsw_sp_kvdl_part_occ(part);
}
-static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
+static u64 mlxsw_sp_kvdl_large_chunks_occ_get(void *priv)
{
- struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+ const struct mlxsw_sp *mlxsw_sp = priv;
struct mlxsw_sp_kvdl_part *part;
part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
return mlxsw_sp_kvdl_part_occ(part);
}
-static const struct devlink_resource_ops mlxsw_sp_kvdl_single_ops = {
- .occ_get = mlxsw_sp_kvdl_single_occ_get,
-};
-
-static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_ops = {
- .occ_get = mlxsw_sp_kvdl_chunks_occ_get,
-};
-
-static const struct devlink_resource_ops mlxsw_sp_kvdl_chunks_large_ops = {
- .occ_get = mlxsw_sp_kvdl_large_chunks_occ_get,
-};
-
int mlxsw_sp_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
{
struct devlink *devlink = priv_to_devlink(mlxsw_core);
@@ -386,8 +372,7 @@ int mlxsw_sp_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
MLXSW_SP_KVDL_SINGLE_SIZE,
MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
MLXSW_SP_RESOURCE_KVD_LINEAR,
- &size_params,
- &mlxsw_sp_kvdl_single_ops);
+ &size_params);
if (err)
return err;
@@ -398,8 +383,7 @@ int mlxsw_sp_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
MLXSW_SP_KVDL_CHUNKS_SIZE,
MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
MLXSW_SP_RESOURCE_KVD_LINEAR,
- &size_params,
- &mlxsw_sp_kvdl_chunks_ops);
+ &size_params);
if (err)
return err;
@@ -410,13 +394,13 @@ int mlxsw_sp_kvdl_resources_register(struct mlxsw_core *mlxsw_core)
MLXSW_SP_KVDL_LARGE_CHUNKS_SIZE,
MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
MLXSW_SP_RESOURCE_KVD_LINEAR,
- &size_params,
- &mlxsw_sp_kvdl_chunks_large_ops);
+ &size_params);
return err;
}
int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
{
+ struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
struct mlxsw_sp_kvdl *kvdl;
int err;
@@ -429,6 +413,23 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
if (err)
goto err_kvdl_parts_init;
+ devlink_resource_occ_get_register(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR,
+ mlxsw_sp_kvdl_occ_get,
+ mlxsw_sp);
+ devlink_resource_occ_get_register(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE,
+ mlxsw_sp_kvdl_single_occ_get,
+ mlxsw_sp);
+ devlink_resource_occ_get_register(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS,
+ mlxsw_sp_kvdl_chunks_occ_get,
+ mlxsw_sp);
+ devlink_resource_occ_get_register(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS,
+ mlxsw_sp_kvdl_large_chunks_occ_get,
+ mlxsw_sp);
+
return 0;
err_kvdl_parts_init:
@@ -438,6 +439,16 @@ int mlxsw_sp_kvdl_init(struct mlxsw_sp *mlxsw_sp)
void mlxsw_sp_kvdl_fini(struct mlxsw_sp *mlxsw_sp)
{
+ struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+
+ devlink_resource_occ_get_unregister(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR_LARGE_CHUNKS);
+ devlink_resource_occ_get_unregister(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR_CHUNKS);
+ devlink_resource_occ_get_unregister(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR_SINGLE);
+ devlink_resource_occ_get_unregister(devlink,
+ MLXSW_SP_RESOURCE_KVD_LINEAR);
mlxsw_sp_kvdl_parts_fini(mlxsw_sp);
kfree(mlxsw_sp->kvdl);
}
diff --git a/drivers/net/netdevsim/devlink.c b/drivers/net/netdevsim/devlink.c
index 1dba47936456..bef7db5d129a 100644
--- a/drivers/net/netdevsim/devlink.c
+++ b/drivers/net/netdevsim/devlink.c
@@ -30,52 +30,36 @@ static struct net *nsim_devlink_net(struct devlink *devlink)
/* IPv4
*/
-static u64 nsim_ipv4_fib_resource_occ_get(struct devlink *devlink)
+static u64 nsim_ipv4_fib_resource_occ_get(void *priv)
{
- struct net *net = nsim_devlink_net(devlink);
+ struct net *net = priv;
return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, false);
}
-static struct devlink_resource_ops nsim_ipv4_fib_res_ops = {
- .occ_get = nsim_ipv4_fib_resource_occ_get,
-};
-
-static u64 nsim_ipv4_fib_rules_res_occ_get(struct devlink *devlink)
+static u64 nsim_ipv4_fib_rules_res_occ_get(void *priv)
{
- struct net *net = nsim_devlink_net(devlink);
+ struct net *net = priv;
return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, false);
}
-static struct devlink_resource_ops nsim_ipv4_fib_rules_res_ops = {
- .occ_get = nsim_ipv4_fib_rules_res_occ_get,
-};
-
/* IPv6
*/
-static u64 nsim_ipv6_fib_resource_occ_get(struct devlink *devlink)
+static u64 nsim_ipv6_fib_resource_occ_get(void *priv)
{
- struct net *net = nsim_devlink_net(devlink);
+ struct net *net = priv;
return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, false);
}
-static struct devlink_resource_ops nsim_ipv6_fib_res_ops = {
- .occ_get = nsim_ipv6_fib_resource_occ_get,
-};
-
-static u64 nsim_ipv6_fib_rules_res_occ_get(struct devlink *devlink)
+static u64 nsim_ipv6_fib_rules_res_occ_get(void *priv)
{
- struct net *net = nsim_devlink_net(devlink);
+ struct net *net = priv;
return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, false);
}
-static struct devlink_resource_ops nsim_ipv6_fib_rules_res_ops = {
- .occ_get = nsim_ipv6_fib_rules_res_occ_get,
-};
-
static int devlink_resources_register(struct devlink *devlink)
{
struct devlink_resource_size_params params = {
@@ -91,7 +75,7 @@ static int devlink_resources_register(struct devlink *devlink)
err = devlink_resource_register(devlink, "IPv4", (u64)-1,
NSIM_RESOURCE_IPV4,
DEVLINK_RESOURCE_ID_PARENT_TOP,
- ¶ms, NULL);
+ ¶ms);
if (err) {
pr_err("Failed to register IPv4 top resource\n");
goto out;
@@ -100,8 +84,7 @@ static int devlink_resources_register(struct devlink *devlink)
n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, true);
err = devlink_resource_register(devlink, "fib", n,
NSIM_RESOURCE_IPV4_FIB,
- NSIM_RESOURCE_IPV4,
- ¶ms, &nsim_ipv4_fib_res_ops);
+ NSIM_RESOURCE_IPV4, ¶ms);
if (err) {
pr_err("Failed to register IPv4 FIB resource\n");
return err;
@@ -110,8 +93,7 @@ static int devlink_resources_register(struct devlink *devlink)
n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, true);
err = devlink_resource_register(devlink, "fib-rules", n,
NSIM_RESOURCE_IPV4_FIB_RULES,
- NSIM_RESOURCE_IPV4,
- ¶ms, &nsim_ipv4_fib_rules_res_ops);
+ NSIM_RESOURCE_IPV4, ¶ms);
if (err) {
pr_err("Failed to register IPv4 FIB rules resource\n");
return err;
@@ -121,7 +103,7 @@ static int devlink_resources_register(struct devlink *devlink)
err = devlink_resource_register(devlink, "IPv6", (u64)-1,
NSIM_RESOURCE_IPV6,
DEVLINK_RESOURCE_ID_PARENT_TOP,
- ¶ms, NULL);
+ ¶ms);
if (err) {
pr_err("Failed to register IPv6 top resource\n");
goto out;
@@ -130,8 +112,7 @@ static int devlink_resources_register(struct devlink *devlink)
n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, true);
err = devlink_resource_register(devlink, "fib", n,
NSIM_RESOURCE_IPV6_FIB,
- NSIM_RESOURCE_IPV6,
- ¶ms, &nsim_ipv6_fib_res_ops);
+ NSIM_RESOURCE_IPV6, ¶ms);
if (err) {
pr_err("Failed to register IPv6 FIB resource\n");
return err;
@@ -140,12 +121,28 @@ static int devlink_resources_register(struct devlink *devlink)
n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, true);
err = devlink_resource_register(devlink, "fib-rules", n,
NSIM_RESOURCE_IPV6_FIB_RULES,
- NSIM_RESOURCE_IPV6,
- ¶ms, &nsim_ipv6_fib_rules_res_ops);
+ NSIM_RESOURCE_IPV6, ¶ms);
if (err) {
pr_err("Failed to register IPv6 FIB rules resource\n");
return err;
}
+
+ devlink_resource_occ_get_register(devlink,
+ NSIM_RESOURCE_IPV4_FIB,
+ nsim_ipv4_fib_resource_occ_get,
+ net);
+ devlink_resource_occ_get_register(devlink,
+ NSIM_RESOURCE_IPV4_FIB_RULES,
+ nsim_ipv4_fib_rules_res_occ_get,
+ net);
+ devlink_resource_occ_get_register(devlink,
+ NSIM_RESOURCE_IPV6_FIB,
+ nsim_ipv6_fib_resource_occ_get,
+ net);
+ devlink_resource_occ_get_register(devlink,
+ NSIM_RESOURCE_IPV6_FIB_RULES,
+ nsim_ipv6_fib_rules_res_occ_get,
+ net);
out:
return err;
}
diff --git a/include/net/devlink.h b/include/net/devlink.h
index e21d8cadd480..2e4f71e16e95 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -231,14 +231,6 @@ struct devlink_dpipe_headers {
unsigned int headers_count;
};
-/**
- * struct devlink_resource_ops - resource ops
- * @occ_get: get the occupied size
- */
-struct devlink_resource_ops {
- u64 (*occ_get)(struct devlink *devlink);
-};
-
/**
* struct devlink_resource_size_params - resource's size parameters
* @size_min: minimum size which can be set
@@ -265,6 +257,8 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
size_params->unit = unit;
}
+typedef u64 devlink_resource_occ_get_t(void *priv);
+
/**
* struct devlink_resource - devlink resource
* @name: name of the resource
@@ -277,7 +271,6 @@ devlink_resource_size_params_init(struct devlink_resource_size_params *size_para
* @size_params: size parameters
* @list: parent list
* @resource_list: list of child resources
- * @resource_ops: resource ops
*/
struct devlink_resource {
const char *name;
@@ -289,7 +282,8 @@ struct devlink_resource {
struct devlink_resource_size_params size_params;
struct list_head list;
struct list_head resource_list;
- const struct devlink_resource_ops *resource_ops;
+ devlink_resource_occ_get_t *occ_get;
+ void *occ_get_priv;
};
#define DEVLINK_RESOURCE_ID_PARENT_TOP 0
@@ -409,8 +403,7 @@ int devlink_resource_register(struct devlink *devlink,
u64 resource_size,
u64 resource_id,
u64 parent_resource_id,
- const struct devlink_resource_size_params *size_params,
- const struct devlink_resource_ops *resource_ops);
+ const struct devlink_resource_size_params *size_params);
void devlink_resources_unregister(struct devlink *devlink,
struct devlink_resource *resource);
int devlink_resource_size_get(struct devlink *devlink,
@@ -419,6 +412,12 @@ int devlink_resource_size_get(struct devlink *devlink,
int devlink_dpipe_table_resource_set(struct devlink *devlink,
const char *table_name, u64 resource_id,
u64 resource_units);
+void devlink_resource_occ_get_register(struct devlink *devlink,
+ u64 resource_id,
+ devlink_resource_occ_get_t *occ_get,
+ void *occ_get_priv);
+void devlink_resource_occ_get_unregister(struct devlink *devlink,
+ u64 resource_id);
#else
@@ -562,8 +561,7 @@ devlink_resource_register(struct devlink *devlink,
u64 resource_size,
u64 resource_id,
u64 parent_resource_id,
- const struct devlink_resource_size_params *size_params,
- const struct devlink_resource_ops *resource_ops)
+ const struct devlink_resource_size_params *size_params)
{
return 0;
}
@@ -589,6 +587,20 @@ devlink_dpipe_table_resource_set(struct devlink *devlink,
return -EOPNOTSUPP;
}
+static inline void
+devlink_resource_occ_get_register(struct devlink *devlink,
+ u64 resource_id,
+ devlink_resource_occ_get_t *occ_get,
+ void *occ_get_priv)
+{
+}
+
+static inline void
+devlink_resource_occ_get_unregister(struct devlink *devlink,
+ u64 resource_id)
+{
+}
+
#endif
#endif /* _NET_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 9236e421bd62..ad1317376798 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2405,6 +2405,16 @@ devlink_resource_size_params_put(struct devlink_resource *resource,
return 0;
}
+static int devlink_resource_occ_put(struct devlink_resource *resource,
+ struct sk_buff *skb)
+{
+ if (!resource->occ_get)
+ return 0;
+ return nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
+ resource->occ_get(resource->occ_get_priv),
+ DEVLINK_ATTR_PAD);
+}
+
static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
struct devlink_resource *resource)
{
@@ -2425,11 +2435,8 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
if (resource->size != resource->size_new)
nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_SIZE_NEW,
resource->size_new, DEVLINK_ATTR_PAD);
- if (resource->resource_ops && resource->resource_ops->occ_get)
- if (nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC,
- resource->resource_ops->occ_get(devlink),
- DEVLINK_ATTR_PAD))
- goto nla_put_failure;
+ if (devlink_resource_occ_put(resource, skb))
+ goto nla_put_failure;
if (devlink_resource_size_params_put(resource, skb))
goto nla_put_failure;
if (list_empty(&resource->resource_list))
@@ -3162,15 +3169,13 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
* @resource_id: resource's id
* @parent_reosurce_id: resource's parent id
* @size params: size parameters
- * @resource_ops: resource ops
*/
int devlink_resource_register(struct devlink *devlink,
const char *resource_name,
u64 resource_size,
u64 resource_id,
u64 parent_resource_id,
- const struct devlink_resource_size_params *size_params,
- const struct devlink_resource_ops *resource_ops)
+ const struct devlink_resource_size_params *size_params)
{
struct devlink_resource *resource;
struct list_head *resource_list;
@@ -3213,7 +3218,6 @@ int devlink_resource_register(struct devlink *devlink,
resource->size = resource_size;
resource->size_new = resource_size;
resource->id = resource_id;
- resource->resource_ops = resource_ops;
resource->size_valid = true;
memcpy(&resource->size_params, size_params,
sizeof(resource->size_params));
@@ -3315,6 +3319,58 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink,
}
EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set);
+/**
+ * devlink_resource_occ_get_register - register occupancy getter
+ *
+ * @devlink: devlink
+ * @resource_id: resource id
+ * @occ_get: occupancy getter callback
+ * @occ_get_priv: occupancy getter callback priv
+ */
+void devlink_resource_occ_get_register(struct devlink *devlink,
+ u64 resource_id,
+ devlink_resource_occ_get_t *occ_get,
+ void *occ_get_priv)
+{
+ struct devlink_resource *resource;
+
+ mutex_lock(&devlink->lock);
+ resource = devlink_resource_find(devlink, NULL, resource_id);
+ if (WARN_ON(!resource))
+ goto out;
+ WARN_ON(resource->occ_get);
+
+ resource->occ_get = occ_get;
+ resource->occ_get_priv = occ_get_priv;
+out:
+ mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
+
+/**
+ * devlink_resource_occ_get_unregister - unregister occupancy getter
+ *
+ * @devlink: devlink
+ * @resource_id: resource id
+ */
+void devlink_resource_occ_get_unregister(struct devlink *devlink,
+ u64 resource_id)
+{
+ struct devlink_resource *resource;
+
+ mutex_lock(&devlink->lock);
+ resource = devlink_resource_find(devlink, NULL, resource_id);
+ if (WARN_ON(!resource))
+ goto out;
+ WARN_ON(!resource->occ_get);
+
+ resource->occ_get = NULL;
+ resource->occ_get_priv = NULL;
+out:
+ mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
+
static int __init devlink_module_init(void)
{
return genl_register_family(&devlink_nl_family);
--
2.14.3
^ permalink raw reply related
* Re: [PATCH 1/2] net: phy: Helper function for reading strapped configuration values
From: Esben Haabendal @ 2018-04-05 20:18 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, open list:ETHERNET PHY LIBRARY, open list,
Rasmus Villemoes
In-Reply-To: <49448b00-e3fd-25df-580e-c02428f1bfe6@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 04/05/2018 04:44 AM, esben.haabendal@gmail.com wrote:
>> From: Esben Haabendal <eha@deif.com>
>>
>> Add a function for use in PHY driver probe functions, reading current
>> autoneg, speed and duplex configuration from BMCR register.
>>
>> Useful for PHY that supports hardware strapped configuration, enabling
>> Linux to respect that configuration (i.e. strapped non-autoneg
>> configuration).
>>
>> Signed-off-by: Esben Haabendal <eha@deif.com>
>> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>> ---
>> drivers/net/phy/phy_device.c | 41 +++++++++++++++++++++++++++++++++++++++++
>> include/linux/phy.h | 1 +
>> 2 files changed, 42 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 74664a6c0cdc..cc52ff2a2344 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1673,6 +1673,47 @@ int genphy_config_init(struct phy_device *phydev)
>> }
>> EXPORT_SYMBOL(genphy_config_init);
>>
>> +/**
>> + * genphy_read_config - read configuration from PHY
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Reads MII_BMCR and sets phydev autoneg, speed and duplex
>> + * accordingly. For use in driver probe functions, to respect strapped
>> + * configuration settings.
>> + */
>> +int genphy_read_config(struct phy_device *phydev)
>
> This duplicates what already exists, in part at least within
> genphy_read_status() can you find a way to use that?
Make a small static function for updating duplex and speed fields from a
BMCR value. It will not be big re-use, but it would make sense. I will
do that in next patch version.
/Esben
^ permalink raw reply
* Re: Enable and configure storm prevention in a network device
From: David Miller @ 2018-04-05 20:20 UTC (permalink / raw)
To: m-karicheri2; +Cc: netdev
In-Reply-To: <76c2b7c7-4f85-c81e-c928-e5650cc26b93@ti.com>
From: Murali Karicheri <m-karicheri2@ti.com>
Date: Thu, 5 Apr 2018 16:14:49 -0400
> Is there a standard way to implement and configure storm prevention
> in a Linux network device?
What kind of "storm", an interrupt storm?
^ permalink raw reply
* Re: [PATCH 2/2] net: phy: dp83640: Read strapped configuration settings
From: Esben Haabendal @ 2018-04-05 20:30 UTC (permalink / raw)
To: Florian Fainelli
Cc: Richard Cochran, Andrew Lunn,
open list:PTP HARDWARE CLOCK SUPPORT, open list, Rasmus Villemoes
In-Reply-To: <95678797-bd17-ba3f-8a70-a00b4792a258@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 04/05/2018 04:44 AM, esben.haabendal@gmail.com wrote:
>> From: Esben Haabendal <eha@deif.com>
>>
>> Read configration settings, to allow automatic forced speed/duplex setup
>> by hardware strapping.
>
> OK but why? What problem is this solving for you?
It is ensuring that the PHY is configured according to the HW design.
If the HW design has set the strap configuration to fx. fixed 100 Mbit
full-duplex, this avoids Linux configuring it for auto-negotiation.
> In general, we do not really want to preserve too much of what the PHY
> has been previously configured with,
Even when this "previous" configuration is the configuration selected by
the board configuration?
> provided that the PHY driver can re-instate these configuration
> values.
This is sort of what I try to do here. The PHY driver needs to check
the BMCR register to figure out what the strapped configuration was.
Without that, it is necessary for software configuration to duplicate
the exact same configuration as is encoded in the strap configuration in
order for the PHY to be configured as it is supposed to.
> I just wonder how this can be robust when you connect this PHY with
> auto-negotiation disabled to a peer that expects a set of link
> parameters not covered by the default advertisement values?
I assume it will fail just as it will if you use ethtool to configure
the PHY wrongly.
> This really looks like a recipe for disaster when you could just
> disable auto-negotiation with ethtool.
The current PHY driver approach to always default to enable
auto-negotation, and then allow user-space to configure it differently
with ethtool.
With this patch, the default configuration is chosen based on the strap
configuration, but user-space can still change the configuration with
ethtool if needed / desired.
I don't think it is such a big change.
Bringing up the PHY with a default configuration according to HW
strapping instead of an in-kernel SW hard-coded value sounds like a good
idea to me.
/Esben
^ permalink raw reply
* Re: [PATCH 1/2] net: phy: Helper function for reading strapped configuration values
From: Esben Haabendal @ 2018-04-05 20:34 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, open list:ETHERNET PHY LIBRARY, open list,
Rasmus Villemoes
In-Reply-To: <49448b00-e3fd-25df-580e-c02428f1bfe6@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 04/05/2018 04:44 AM, esben.haabendal@gmail.com wrote:
>> From: Esben Haabendal <eha@deif.com>
>>
>> Add a function for use in PHY driver probe functions, reading current
>> autoneg, speed and duplex configuration from BMCR register.
>>
>> Useful for PHY that supports hardware strapped configuration, enabling
>> Linux to respect that configuration (i.e. strapped non-autoneg
>> configuration).
>>
>> Signed-off-by: Esben Haabendal <eha@deif.com>
>> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>> ---
>> drivers/net/phy/phy_device.c | 41 +++++++++++++++++++++++++++++++++++++++++
>> include/linux/phy.h | 1 +
>> 2 files changed, 42 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 74664a6c0cdc..cc52ff2a2344 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1673,6 +1673,47 @@ int genphy_config_init(struct phy_device *phydev)
>> }
>> EXPORT_SYMBOL(genphy_config_init);
>>
>> +/**
>> + * genphy_read_config - read configuration from PHY
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Reads MII_BMCR and sets phydev autoneg, speed and duplex
>> + * accordingly. For use in driver probe functions, to respect strapped
>> + * configuration settings.
>> + */
>> +int genphy_read_config(struct phy_device *phydev)
>
> This duplicates what already exists, in part at least within
> genphy_read_status() can you find a way to use that?
Make a small static function for updating duplex and speed fields from a
BMCR value. It will not be big re-use, but it would make sense. I will
do that in next patch version.
/Esben
^ permalink raw reply
* Re: [PATCH 2/2] net: phy: dp83640: Read strapped configuration settings
From: Esben Haabendal @ 2018-04-05 20:34 UTC (permalink / raw)
To: Florian Fainelli
Cc: Richard Cochran, Andrew Lunn,
open list:PTP HARDWARE CLOCK SUPPORT, open list, Rasmus Villemoes
In-Reply-To: <95678797-bd17-ba3f-8a70-a00b4792a258@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 04/05/2018 04:44 AM, esben.haabendal@gmail.com wrote:
>> From: Esben Haabendal <eha@deif.com>
>>
>> Read configration settings, to allow automatic forced speed/duplex setup
>> by hardware strapping.
>
> OK but why? What problem is this solving for you?
It is ensuring that the PHY is configured according to the HW design.
If the HW design has set the strap configuration to fx. fixed 100 Mbit
full-duplex, this avoids Linux configuring it for auto-negotiation.
> In general, we do not really want to preserve too much of what the PHY
> has been previously configured with,
Even when this "previous" configuration is the configuration selected by
the board configuration?
> provided that the PHY driver can re-instate these configuration
> values.
This is sort of what I try to do here. The PHY driver needs to check
the BMCR register to figure out what the strapped configuration was.
Without that, it is necessary for software configuration to duplicate
the exact same configuration as is encoded in the strap configuration in
order for the PHY to be configured as it is supposed to.
> I just wonder how this can be robust when you connect this PHY with
> auto-negotiation disabled to a peer that expects a set of link
> parameters not covered by the default advertisement values?
I assume it will fail just as it will if you use ethtool to configure
the PHY wrongly.
> This really looks like a recipe for disaster when you could just
> disable auto-negotiation with ethtool.
The current PHY driver approach to always default to enable
auto-negotation, and then allow user-space to configure it differently
with ethtool.
With this patch, the default configuration is chosen based on the strap
configuration, but user-space can still change the configuration with
ethtool if needed / desired.
I don't think it is such a big change.
Bringing up the PHY with a default configuration according to HW
strapping instead of an in-kernel SW hard-coded value sounds like a good
idea to me.
/Esben
^ permalink raw reply
* [PATCH v2] net: phy: marvell: Enable interrupt function on LED2 pin
From: Esben Haabendal @ 2018-04-05 20:40 UTC (permalink / raw)
To: netdev
Cc: Esben Haabendal, Rasmus Villemoes, Andrew Lunn, Florian Fainelli,
open list
In-Reply-To: <20180405133504.12257-1-esben.haabendal@gmail.com>
From: Esben Haabendal <eha@deif.com>
The LED2[2]/INTn pin on Marvell 88E1318S as well as 88E1510/12/14/18 needs
to be configured to be usable as interrupt not only when WOL is enabled,
but whenever we rely on interrupts from the PHY.
Signed-off-by: Esben Haabendal <eha@deif.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
drivers/net/phy/marvell.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0e0978d8a0eb..a6ad0255c512 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -828,6 +828,22 @@ static int m88e1121_config_init(struct phy_device *phydev)
return marvell_config_init(phydev);
}
+static int m88e1318_config_init(struct phy_device *phydev)
+{
+ if (phy_interrupt_is_valid(phydev)) {
+ int err = phy_modify_paged(
+ phydev, MII_MARVELL_LED_PAGE,
+ MII_88E1318S_PHY_LED_TCR,
+ MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+ MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+ MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
+ if (err < 0)
+ return err;
+ }
+
+ return m88e1121_config_init(phydev);
+}
+
static int m88e1510_config_init(struct phy_device *phydev)
{
int err;
@@ -870,7 +886,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
phydev->advertising &= ~pause;
}
- return m88e1121_config_init(phydev);
+ return m88e1318_config_init(phydev);
}
static int m88e1118_config_aneg(struct phy_device *phydev)
@@ -2086,7 +2102,7 @@ static struct phy_driver marvell_drivers[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.probe = marvell_probe,
- .config_init = &m88e1121_config_init,
+ .config_init = &m88e1318_config_init,
.config_aneg = &m88e1318_config_aneg,
.read_status = &marvell_read_status,
.ack_interrupt = &marvell_ack_interrupt,
--
2.16.3
^ permalink raw reply related
* Re: [PATCH 2/2] net: phy: dp83640: Read strapped configuration settings
From: Andrew Lunn @ 2018-04-05 20:40 UTC (permalink / raw)
To: Esben Haabendal
Cc: Florian Fainelli, Richard Cochran,
open list:PTP HARDWARE CLOCK SUPPORT, open list, Rasmus Villemoes
In-Reply-To: <87d0zdjszu.fsf@haabendal.dk>
On Thu, Apr 05, 2018 at 10:30:45PM +0200, Esben Haabendal wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
> > On 04/05/2018 04:44 AM, esben.haabendal@gmail.com wrote:
> >> From: Esben Haabendal <eha@deif.com>
> >>
> >> Read configration settings, to allow automatic forced speed/duplex setup
> >> by hardware strapping.
> >
> > OK but why? What problem is this solving for you?
>
> It is ensuring that the PHY is configured according to the HW design.
> If the HW design has set the strap configuration to fx. fixed 100 Mbit
> full-duplex, this avoids Linux configuring it for auto-negotiation.
Hi Esben
Are you sure it contains the HW strapping? Is the driver hitting the
PHY with a hard reset to make it go back to the strapping defaults?
Or could it still contain whatever state the last boot of Linux, or
maybe the bootloader, left the PHY in?
Andrew
^ permalink raw reply
* Re: marvell switch
From: Andrew Lunn @ 2018-04-05 20:46 UTC (permalink / raw)
To: Ran Shalit; +Cc: netdev
In-Reply-To: <CAJ2oMhL4=ZrkGaTgTXU-oJFhZP==QDkO4fNSBaaTJF+NHnf9-g@mail.gmail.com>
> > Hi Ran
> >
> > The Marvell driver makes each port act like a normal Linux network
> > interface. So if you want to enable a port, do
> >
> > ip link set lan0 up
> >
> > Want to add an ip address to a port
> >
> > ip addr add 10.42.42.42/24 dev lan0
> >
> > Want to bridge two ports
> >
> > ip link add name br0 type bridge
> > ip link set dev br0 up
> > ip link set dev lan0 master br0
> > ip link set dev lan1 master br0
> >
> > Just treat them as normal interfaces.
> >
>
> If I may please ask,
> What is the purpose of using bridge for configuring switch interfaces.
> Is it in order to isolate some ports from others?
> I ask because according to my understanding the default configuration of
> the driver is to enable switch in "flat" configuration, i.e. as if all
> ports are connected to each other.
Please think about what i said. They are standard Linux network
interfaces. Do standard Linux network interfaces bridge themselves
together by default? No, you need to configure a bridge.
Andrew
^ 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