* Re: What to do when a bridge port gets its pvid deleted?
From: Vladimir Oltean @ 2019-08-19 21:10 UTC (permalink / raw)
To: Ido Schimmel
Cc: Florian Fainelli, Roopa Prabhu, nikolay, netdev, Andrew Lunn,
Vivien Didelot, stephen
In-Reply-To: <20190819201502.GA25207@splinter>
On Mon, 19 Aug 2019 at 23:15, Ido Schimmel <idosch@idosch.org> wrote:
>
> On Mon, Aug 19, 2019 at 08:15:03PM +0300, Vladimir Oltean wrote:
> > On 6/28/19 7:45 PM, Florian Fainelli wrote:
> > > On 6/28/19 5:37 AM, Vladimir Oltean wrote:
> > > > On Fri, 28 Jun 2019 at 15:30, Ido Schimmel <idosch@idosch.org> wrote:
> > > > >
> > > > > On Tue, Jun 25, 2019 at 11:49:29PM +0300, Vladimir Oltean wrote:
> > > > > > A number of DSA drivers (BCM53XX, Microchip KSZ94XX, Mediatek MT7530
> > > > > > at the very least), as well as Mellanox Spectrum (I didn't look at all
> > > > > > the pure switchdev drivers) try to restore the pvid to a default value
> > > > > > on .port_vlan_del.
> > > > >
> > > > > I don't know about DSA drivers, but that's not what mlxsw is doing. If
> > > > > the VLAN that is configured as PVID is deleted from the bridge port, the
> > > > > driver instructs the port to discard untagged and prio-tagged packets.
> > > > > This is consistent with the bridge driver's behavior.
> > > > >
> > > > > We do have a flow the "restores" the PVID, but that's when a port is
> > > > > unlinked from its bridge master. The PVID we set is 4095 which cannot be
> > > > > configured by the 8021q / bridge driver. This is due to the way the
> > > > > underlying hardware works. Even if a port is not bridged and used purely
> > > > > for routing, packets still do L2 lookup first which sends them directly
> > > > > to the router block. If PVID is not configured, untagged packets could
> > > > > not be routed. Obviously, at egress we strip this VLAN.
> > > > >
> > > > > > Sure, the port stops receiving traffic when its pvid is a VLAN ID that
> > > > > > is not installed in its hw filter, but as far as the bridge core is
> > > > > > concerned, this is to be expected:
> > > > > >
> > > > > > # bridge vlan add dev swp2 vid 100 pvid untagged
> > > > > > # bridge vlan
> > > > > > port vlan ids
> > > > > > swp5 1 PVID Egress Untagged
> > > > > >
> > > > > > swp2 1 Egress Untagged
> > > > > > 100 PVID Egress Untagged
> > > > > >
> > > > > > swp3 1 PVID Egress Untagged
> > > > > >
> > > > > > swp4 1 PVID Egress Untagged
> > > > > >
> > > > > > br0 1 PVID Egress Untagged
> > > > > > # ping 10.0.0.1
> > > > > > PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.
> > > > > > 64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=0.682 ms
> > > > > > 64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=0.299 ms
> > > > > > 64 bytes from 10.0.0.1: icmp_seq=3 ttl=64 time=0.251 ms
> > > > > > 64 bytes from 10.0.0.1: icmp_seq=4 ttl=64 time=0.324 ms
> > > > > > 64 bytes from 10.0.0.1: icmp_seq=5 ttl=64 time=0.257 ms
> > > > > > ^C
> > > > > > --- 10.0.0.1 ping statistics ---
> > > > > > 5 packets transmitted, 5 received, 0% packet loss, time 4188ms
> > > > > > rtt min/avg/max/mdev = 0.251/0.362/0.682/0.163 ms
> > > > > > # bridge vlan del dev swp2 vid 100
> > > > > > # bridge vlan
> > > > > > port vlan ids
> > > > > > swp5 1 PVID Egress Untagged
> > > > > >
> > > > > > swp2 1 Egress Untagged
> > > > > >
> > > > > > swp3 1 PVID Egress Untagged
> > > > > >
> > > > > > swp4 1 PVID Egress Untagged
> > > > > >
> > > > > > br0 1 PVID Egress Untagged
> > > > > >
> > > > > > # ping 10.0.0.1
> > > > > > PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.
> > > > > > ^C
> > > > > > --- 10.0.0.1 ping statistics ---
> > > > > > 8 packets transmitted, 0 received, 100% packet loss, time 7267ms
> > > > > >
> > > > > > What is the consensus here? Is there a reason why the bridge driver
> > > > > > doesn't take care of this?
> > > > >
> > > > > Take care of what? :) Always maintaining a PVID on the bridge port? It's
> > > > > completely OK not to have a PVID.
> > > > >
> > > >
> > > > Yes, I didn't think it through during the first email. I came to the
> > > > same conclusion in the second one.
> > > >
> > > > > > Do switchdev drivers have to restore the pvid to always be
> > > > > > operational, even if their state becomes inconsistent with the upper
> > > > > > dev? Is it just 'nice to have'? What if VID 1 isn't in the hw filter
> > > > > > either (perfectly legal)?
> > > > >
> > > > > Are you saying that DSA drivers always maintain a PVID on the bridge
> > > > > port and allow untagged traffic to ingress regardless of the bridge
> > > > > driver's configuration? If so, I think this needs to be fixed.
> > > >
> > > > Well, not at the DSA core level.
> > > > But for Microchip:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/microchip/ksz9477.c#n576
> > > > For Broadcom:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/b53/b53_common.c#n1376
> > > > For Mediatek:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/mt7530.c#n1196
> > > >
> > > > There might be others as well.
> > >
> > > That sounds bogus indeed, and I bet that the two other drivers just
> > > followed the b53 driver there. I will have to test this again and come
> > > up with a patch eventually.
> > >
> > > When the port leaves the bridge we do bring it back into a default PVID
> > > (which is different than the bridge's default PVID) so that part should
> > > be okay.
> > >
> >
> > Adding a few more networking people.
> > So my flow is something like this:
> > - Boot a board with a DSA switch
> > - Bring all interfaces up
> > - Enslave all interfaces to br0
> > - Enable vlan_filtering on br0
> >
> > What VIDs should be installed into the ports' hw filters, and what should
> > the pvid be at this point?
> > Should the switch ports pass any traffic?
> > At this point, 'bridge vlan' shows a confusing:
> > port vlan ids
> > eth0 1 PVID Egress Untagged
> >
> > swp5 1 PVID Egress Untagged
> >
> > swp2 1 PVID Egress Untagged
> >
> > swp3 1 PVID Egress Untagged
> >
> > swp4 1 PVID Egress Untagged
> >
> > br0 1 PVID Egress Untagged
> > for all ports, but the .port_vlan_add callback is nowhere to be found.
>
> The bridge adds a PVID on the port when it is enslaved to the bridge.
> The configuration only takes effect when VLAN filtering is enabled. I'm
> looking at dsa_port_vlan_add() and it seems that it does not propagate
> the VLAN call when VLAN filtering is disabled. This explains why you
> never see the callback.
>
Aha! The offending commit is this:
commit 2ea7a679ca2abd251c1ec03f20508619707e1749
Author: Andrew Lunn <andrew@lunn.ch>
Date: Tue Nov 7 00:04:24 2017 +0100
net: dsa: Don't add vlans when vlan filtering is disabled
The software bridge can be build with vlan filtering support
included. However, by default it is turned off. In its turned off
state, it still passes VLANs via switchev, even though they are not to
be used. Don't pass these VLANs to the hardware. Only do so when vlan
filtering is enabled.
This fixes at least one corner case. There are still issues in other
corners, such as when vlan_filtering is later enabled.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
It's good to know that it's there (like you said, it explains some
things) but I can't exactly say that removing it helps in any way.
In fact, removing it only overwrites the dsa_8021q VLANs with 1 during
bridge_join, while not actually doing anything upon a vlan_filtering
toggle.
So the sja1105 driver is in a way shielded by DSA from the bridge, for
the better.
It still appears to be true that the bridge doesn't think it's
necessary to notify through SWITCHDEV_OBJ_ID_PORT_VLAN again. So my
best bet is to restore the pvid on my own.
However I've already stumbled upon an apparent bug while trying to do
that. Does this look off? If it doesn't, I'll submit it as a patch:
commit 788f03991aa576fc0b4b26ca330af0f412c55582
Author: Vladimir Oltean <olteanv@gmail.com>
Date: Mon Aug 19 22:57:00 2019 +0300
net: bridge: Keep the BRIDGE_VLAN_INFO_PVID flag in net_bridge_vlan
Currently this simplified code snippet fails:
br_vlan_get_pvid(netdev, &pvid);
br_vlan_get_info(netdev, pvid, &vinfo);
ASSERT(!(vinfo.flags & BRIDGE_VLAN_INFO_PVID));
It is intuitive that the pvid of a netdevice should have the
BRIDGE_VLAN_INFO_PVID flag set.
However I can't seem to pinpoint a commit where this behavior was
introduced. It seems like it's been like that since forever.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 021cc9f66804..f49b2758bcab 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -68,10 +68,13 @@ static bool __vlan_add_flags(struct
net_bridge_vlan *v, u16 flags)
else
vg = nbp_vlan_group(v->port);
- if (flags & BRIDGE_VLAN_INFO_PVID)
+ if (flags & BRIDGE_VLAN_INFO_PVID) {
ret = __vlan_add_pvid(vg, v->vid);
- else
+ v->flags |= BRIDGE_VLAN_INFO_PVID;
+ } else {
ret = __vlan_delete_pvid(vg, v->vid);
+ v->flags &= ~BRIDGE_VLAN_INFO_PVID;
+ }
if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
> I assume that if you configure the bridge with VLAN filtering enabled
> and then enslave a port, then everything works fine.
>
> mlxsw avoids the situation by forbidding the toggling of VLAN filtering
> on the bridge when its ports are enslaved.
>
I can certainly understand where this comes from. However a simpleton
might object that this:
ip link add name br0 type bridge vlan_filtering 1
ip link set dev swp2 master br0
should behave the same as this:
ip link add name br0 type bridge
ip link set dev swp2 master br0
echo 1 > /sys/class/net/br0/bridge/vlan_filtering
I can't disagree with said simpleton.
> >
> > Whose responsibility is it for the switch to pass traffic without any
> > further 'bridge vlan' command? What is the mechanism through which this
> > should work?
> >
> > What if I do:
> > sudo bridge vlan add vid 100 dev swp2 pvid untagged
> > echo 0 | sudo tee /sys/class/net/br0/bridge/vlan_filtering
> > echo 1 | sudo tee /sys/class/net/br0/bridge/vlan_filtering
> > What pvid should there be on swp2 now?
> > 'bridge vlan' shows:
> > port vlan ids
> > eth0 1 PVID Egress Untagged
> >
> > swp5 1 PVID Egress Untagged
> >
> > swp2 1 Egress Untagged
> > 100 PVID Egress Untagged
> >
> > swp3 1 PVID Egress Untagged
> >
> > swp4 1 PVID Egress Untagged
> >
> > br0 1 PVID Egress Untagged
> > If the 'bridge vlan' output is correct, whose responsibility is it to
> > restore this pvid?
>
> I suggest to follow mlxsw and avoid this mess. You can support both VLAN
> filtering enable / disable without supporting dynamically toggling the
> option.
>
> >
> > More context: the sja1105 driver is somewhat similar to the mlxsw in that
> > VLAN awareness cannot be truly disabled. Arid details aside, in both cases,
> > achieving "VLAN-unaware"-like behavior involves manipulating the pvid in
> > both cases. But it appears that the bridge core does expect:
> > (1) that the driver performs a default VLAN initialization which matches its
> > own, without them ever communicating. But because switchdev/DSA drivers
> > start off in standalone mode, vlan_filtering=0 comes first, hence the
> > non-standard pvid. Through what mechanism is the bridge-expected pvid
> > supposed to get restored upon flipping vlan_filtering?
> > (2) that toggling VLAN filtering off and on has no other state upon the
> > underlying driver than enabling and disabling VLAN awareness. The VLAN hw
> > filter table is assumed to be unchanged. Is this a correct assumption?
> >
> > Thanks,
> > -Vladimir
^ permalink raw reply related
* Re: INFO: task hung in tls_sw_sendmsg
From: Jakub Kicinski @ 2019-08-19 21:09 UTC (permalink / raw)
To: syzbot
Cc: ast, aviadye, borisp, bpf, daniel, davejwatson, davem,
john.fastabend, kafai, linux-kernel, netdev, songliubraving,
syzkaller-bugs, yhs, Eric Biggers, Steffen Klassert, linux-crypto
In-Reply-To: <0000000000006a71990583cd3d9c@google.com>
On Mon, 11 Mar 2019 01:20:05 -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: d9862cfb Merge tag 'mips_5.1' of git://git.kernel.org/pub/..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=16e9d977200000
> kernel config: https://syzkaller.appspot.com/x/.config?x=73d88a42238825ad
> dashboard link: https://syzkaller.appspot.com/bug?extid=8a6df99c3b1812093b70
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> userspace arch: amd64
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+8a6df99c3b1812093b70@syzkaller.appspotmail.com
Looks like the TLS bugs which involve pcrypt may be a pcrypt issue.
There seems to be no clear explanation for these in TLS code.
#syz dup: INFO: task hung in aead_recvmsg
This is similar to:
https://syzkaller.appspot.com/bug?id=44ae4b4fa7e6c6e92aa921d2ec20ce9fbee97939
(INFO: task hung in tls_sw_free_resources_tx)
^ permalink raw reply
* Re: [PATCH V5 0/9] Fixes for vhost metadata acceleration
From: Michael S. Tsirkin @ 2019-08-19 21:08 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, virtualization, netdev, linux-kernel, linux-mm, jgg
In-Reply-To: <663be71f-f96d-cfbc-95a0-da0ac6b82d9f@redhat.com>
On Tue, Aug 13, 2019 at 04:12:49PM +0800, Jason Wang wrote:
>
> On 2019/8/12 下午5:49, Michael S. Tsirkin wrote:
> > On Mon, Aug 12, 2019 at 10:44:51AM +0800, Jason Wang wrote:
> > > On 2019/8/11 上午1:52, Michael S. Tsirkin wrote:
> > > > On Fri, Aug 09, 2019 at 01:48:42AM -0400, Jason Wang wrote:
> > > > > Hi all:
> > > > >
> > > > > This series try to fix several issues introduced by meta data
> > > > > accelreation series. Please review.
> > > > >
> > > > > Changes from V4:
> > > > > - switch to use spinlock synchronize MMU notifier with accessors
> > > > >
> > > > > Changes from V3:
> > > > > - remove the unnecessary patch
> > > > >
> > > > > Changes from V2:
> > > > > - use seqlck helper to synchronize MMU notifier with vhost worker
> > > > >
> > > > > Changes from V1:
> > > > > - try not use RCU to syncrhonize MMU notifier with vhost worker
> > > > > - set dirty pages after no readers
> > > > > - return -EAGAIN only when we find the range is overlapped with
> > > > > metadata
> > > > >
> > > > > Jason Wang (9):
> > > > > vhost: don't set uaddr for invalid address
> > > > > vhost: validate MMU notifier registration
> > > > > vhost: fix vhost map leak
> > > > > vhost: reset invalidate_count in vhost_set_vring_num_addr()
> > > > > vhost: mark dirty pages during map uninit
> > > > > vhost: don't do synchronize_rcu() in vhost_uninit_vq_maps()
> > > > > vhost: do not use RCU to synchronize MMU notifier with worker
> > > > > vhost: correctly set dirty pages in MMU notifiers callback
> > > > > vhost: do not return -EAGAIN for non blocking invalidation too early
> > > > >
> > > > > drivers/vhost/vhost.c | 202 +++++++++++++++++++++++++-----------------
> > > > > drivers/vhost/vhost.h | 6 +-
> > > > > 2 files changed, 122 insertions(+), 86 deletions(-)
> > > > This generally looks more solid.
> > > >
> > > > But this amounts to a significant overhaul of the code.
> > > >
> > > > At this point how about we revert 7f466032dc9e5a61217f22ea34b2df932786bbfc
> > > > for this release, and then re-apply a corrected version
> > > > for the next one?
> > >
> > > If possible, consider we've actually disabled the feature. How about just
> > > queued those patches for next release?
> > >
> > > Thanks
> > Sorry if I was unclear. My idea is that
> > 1. I revert the disabled code
> > 2. You send a patch readding it with all the fixes squashed
> > 3. Maybe optimizations on top right away?
> > 4. We queue *that* for next and see what happens.
> >
> > And the advantage over the patchy approach is that the current patches
> > are hard to review. E.g. it's not reasonable to ask RCU guys to review
> > the whole of vhost for RCU usage but it's much more reasonable to ask
> > about a specific patch.
>
>
> Ok. Then I agree to revert.
>
> Thanks
Great, so please send the following:
- revert
- squashed and fixed patch
--
MST
^ permalink raw reply
* RE: [PATCH] net/ncsi: add control packet payload to NC-SI commands from netlink
From: Ben Wei @ 2019-08-19 20:56 UTC (permalink / raw)
To: Justin.Lee1@Dell.com
Cc: netdev@vger.kernel.org, openbmc@lists.ozlabs.org,
linux-kernel@vger.kernel.org, sam@mendozajonas.com,
davem@davemloft.net, Deepak Kodihalli
In-Reply-To: <ec5842fff4de45069a51618eb72df164@AUSX13MPS302.AMER.DELL.COM>
Hi Justin,
> Hi Ben,
>
> I have similar fix locally with different approach as the command handler may have some expectation for those byes.
> We can use NCSI_PKT_CMD_OEM handler as it only copies data based on the payload length.
Great! Yes I was thinking the same, we just need some way to take data payload sent from netlink message and sent it over NC-SI.
>
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c index 5c3fad8..3b01f65 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -309,14 +309,19 @@ static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca)
>
> int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) {
> + struct ncsi_cmd_handler *nch = NULL;
> struct ncsi_request *nr;
> + unsigned char type;
> struct ethhdr *eh;
> - struct ncsi_cmd_handler *nch = NULL;
> int i, ret;
>
> + if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN)
> + type = NCSI_PKT_CMD_OEM;
> + else
> + type = nca->type;
> /* Search for the handler */
> for (i = 0; i < ARRAY_SIZE(ncsi_cmd_handlers); i++) {
> - if (ncsi_cmd_handlers[i].type == nca->type) {
> + if (ncsi_cmd_handlers[i].type == type) {
> if (ncsi_cmd_handlers[i].handler)
> nch = &ncsi_cmd_handlers[i];
> else
>
So in this case NCSI_PKT_CMD_OEM would be the default handler for all NC-SI command over netlink (standard and OEM), correct?
Should we rename this to something like NCSI_PKT_CMD_GENERIC for clarity perhaps? Do you plan to upstream this patch?
Also do you have local patch to support NCSI_PKT_CMD_PLDM and the PLDM over NC-SI commands defined here (https://www.dmtf.org/sites/default/files/NC-SI_1.2_PLDM_Support_over_RBT_Commands_Proposal.pdf)?
If not I can send my local changes - but I think we can use the same NCSI_PKT_CMD_OEM handler to transport PLDM payload over NC-SI.
What do you think?
(CC Deepak as I think once this is in place we can use pldmtool to send basic PLDM payloads over NC-SI)
Regards,
-Ben
^ permalink raw reply
* Re: [PATCH net-next 2/3] tcp: ulp: add functions to dump ulp-specific information
From: Jakub Kicinski @ 2019-08-19 20:40 UTC (permalink / raw)
To: Davide Caratti
Cc: Eric Dumazet, Boris Pismenny, John Fastabend, Dave Watson,
Aviad Yehezkel, David S. Miller, netdev
In-Reply-To: <b765aa08456ef258615a46e7ff106703a240ddb5.camel@redhat.com>
On Mon, 19 Aug 2019 15:32:09 +0200, Davide Caratti wrote:
> On Thu, 2019-08-15 at 14:38 -0700, Jakub Kicinski wrote:
> > On Thu, 15 Aug 2019 20:46:01 +0200, Eric Dumazet wrote:
> > > On 8/15/19 6:00 PM, Davide Caratti wrote:
> > > > + if (net_admin) {
> > > > + const struct tcp_ulp_ops *ulp_ops;
> > > > +
> > > > + rcu_read_lock();
> > > > + ulp_ops = icsk->icsk_ulp_ops;
> > > > + if (ulp_ops)
> > > > + err = tcp_diag_put_ulp(skb, sk, ulp_ops);
> > > > + rcu_read_unlock();
> > > > + if (err)
> > > > + return err;
> > > > + }
> > > > return 0;
> > >
> > > Why is rcu_read_lock() and rcu_read_unlock() used at all ?
> > >
> > > icsk->icsk_ulp_ops does not seem to be rcu protected ?
> > >
> > > If this was, then an rcu_dereference() would be appropriate.
> >
> > Indeed it's ulp_data not ulp_ops that are protected.
>
> the goal is to protect execution of 'ss -tni' against concurrent removal
> of tls.ko module, similarly to what was done in inet_sk_diag_fill() when
> INET_DIAG_CONG is requested [1]. But after reading more carefully, the
> assignment of ulp_ops needs to be:
>
> ulp_ops = READ_ONCE(icsk->icsk_ulp_ops);
>
> which I lost in internal reviews, with some additional explanatory
> comment. Ok if I correct the above hunk with READ_ONCE() and add a
> comment?
Seems like a forth while future-proofing. Currently the ULP can't
change, and is only released when socket is destroyed, so we should
be safe (unlike CC which can be changed at any moment).
We should mark the pointer as RCU tho, I find it hard to wrap my head
around these half-way RCU pointers with just READ_ONCE() on them :S
> > Davide, perhaps we could push the RCU lock into tls_get_info(), after all?
>
> It depends on whether concurrent dump / module removal is an issue for TCP
> ULPs, like it was for congestion control schemes [1]. Any advice?
If we're willing to mark icsk->icsk_ulp_ops as RCU I think it's fine.
But I'm not 100% sure its worth the churn :S
> > And tls_context has to use rcu_deference there, as Eric points out,
> > plus we should probably NULL-check it.
>
> yes, it makes sense, for patch 3/3, in the assignment of 'ctx'. Instead of
> calling tls_get_ctx() in tls_get_info() I will do
>
> ctx = rcu_dereference(inet_csk(sk)->icsk_ulp_data);
>
> and let it return 0 in case of NULL ctx (as it doesn't look like a faulty
> situation). Ok?
SGTM!
^ permalink raw reply
* Re: [PATCH net] nfp: flower: verify that block cb is not busy before binding
From: Jakub Kicinski @ 2019-08-19 20:29 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem, pablo
In-Reply-To: <20190819073304.9419-1-vladbu@mellanox.com>
On Mon, 19 Aug 2019 10:33:04 +0300, Vlad Buslov wrote:
> When processing FLOW_BLOCK_BIND command on indirect block, check that flow
> block cb is not busy.
>
> Fixes: 0d4fd02e7199 ("net: flow_offload: add flow_block_cb_is_busy() and use it")
> Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thank you!
^ permalink raw reply
* Re: [PATCH net-next v7 5/6] flow_offload: support get multi-subsystem block
From: Jakub Kicinski @ 2019-08-19 20:27 UTC (permalink / raw)
To: Vlad Buslov
Cc: wenxu, David Miller, Jiri Pirko, pablo@netfilter.org,
netfilter-devel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <vbftvady5tg.fsf@mellanox.com>
On Mon, 19 Aug 2019 07:26:07 +0000, Vlad Buslov wrote:
> On Fri 16 Aug 2019 at 20:56, Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> > Hi Vlad!
> >
> > While looking into this, would you mind also add the missing
> > flow_block_cb_is_busy() calls in the indirect handlers in the drivers?
> >
> > LMK if you're too busy, I don't want this to get forgotten :)
>
> Hi Jakub,
>
> I've checked the code and it looks like only nfp driver is affected:
>
> - I added check in nfp to lookup cb_priv with
> nfp_flower_indr_block_cb_priv_lookup() and call
> flow_block_cb_is_busy() if cb_priv exists.
>
> - In mlx5 en_rep.c there is already a check that indr_priv exists, so
> trying to lookup block_cb->cb_indent==indr_priv is redundant.
>
> - Switch drivers (mlxsw and ocelot) take reference to block_cb on
> FLOW_BLOCK_BIND, so they should not require any modifications.
>
> Tell me if I missed anything. Sending the patch for nfp.
Ah, that sounds plausible, I've only checked the nfp driver.
^ permalink raw reply
* RE: [PATCH] net/ncsi: add control packet payload to NC-SI commands from netlink
From: Justin.Lee1 @ 2019-08-19 20:17 UTC (permalink / raw)
To: benwei; +Cc: netdev, openbmc, linux-kernel, sam, davem
In-Reply-To: <CH2PR15MB368691D280F882864A6D356DA3A80@CH2PR15MB3686.namprd15.prod.outlook.com>
Hi Ben,
I have similar fix locally with different approach as the command handler may have some expectation for those byes.
We can use NCSI_PKT_CMD_OEM handler as it only copies data based on the payload length.
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 5c3fad8..3b01f65 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -309,14 +309,19 @@ static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca)
int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
{
+ struct ncsi_cmd_handler *nch = NULL;
struct ncsi_request *nr;
+ unsigned char type;
struct ethhdr *eh;
- struct ncsi_cmd_handler *nch = NULL;
int i, ret;
+ if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN)
+ type = NCSI_PKT_CMD_OEM;
+ else
+ type = nca->type;
/* Search for the handler */
for (i = 0; i < ARRAY_SIZE(ncsi_cmd_handlers); i++) {
- if (ncsi_cmd_handlers[i].type == nca->type) {
+ if (ncsi_cmd_handlers[i].type == type) {
if (ncsi_cmd_handlers[i].handler)
nch = &ncsi_cmd_handlers[i];
else
Thanks,
Justin
> This patch adds support for NCSI_CMD_SEND_CMD netlink command to load packet data payload (up to 16 bytes) to standard NC-SI commands.
>
> Packet data will be loaded from NCSI_ATTR_DATA attribute similar to NC-SI OEM commands
>
> Signed-off-by: Ben Wei <benwei@fb.com>
> ---
> net/ncsi/internal.h | 7 ++++---
> net/ncsi/ncsi-netlink.c | 9 +++++++++
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h index 0b3f0673e1a2..4ff442faf5dc 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -328,9 +328,10 @@ struct ncsi_cmd_arg {
> unsigned short payload; /* Command packet payload length */
> unsigned int req_flags; /* NCSI request properties */
> union {
> - unsigned char bytes[16]; /* Command packet specific data */
> - unsigned short words[8];
> - unsigned int dwords[4];
> +#define NCSI_MAX_DATA_BYTES 16
> + unsigned char bytes[NCSI_MAX_DATA_BYTES]; /* Command packet specific data */
> + unsigned short words[NCSI_MAX_DATA_BYTES / sizeof(unsigned short)];
> + unsigned int dwords[NCSI_MAX_DATA_BYTES / sizeof(unsigned int)];
> };
> unsigned char *data; /* NCSI OEM data */
> struct genl_info *info; /* Netlink information */
> diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c index 8b386d766e7d..7d2a43f30eb1 100644
> --- a/net/ncsi/ncsi-netlink.c
> +++ b/net/ncsi/ncsi-netlink.c
> @@ -459,6 +459,15 @@ static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
> nca.payload = ntohs(hdr->length);
> nca.data = data + sizeof(*hdr);
>
> + if (nca.payload <= NCSI_MAX_DATA_BYTES) {
> + memcpy(nca.bytes, nca.data, nca.payload);
> + } else {
> + netdev_info(ndp->ndev.dev, "NCSI:payload size %u exceeds max %u\n",
> + nca.payload, NCSI_MAX_DATA_BYTES);
> + ret = -EINVAL;
> + goto out_netlink;
> + }
> +
> ret = ncsi_xmit_cmd(&nca);
> out_netlink:
> if (ret != 0) {
> --
> 2.17.1
^ permalink raw reply related
* Re: [PATCH 0/5] Netfilter fixes for net
From: David Miller @ 2019-08-19 20:16 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20190819184911.15263-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 19 Aug 2019 20:49:06 +0200
> The following patchset contains Netfilter fixes for net:
>
> 1) Remove IP MASQUERADING record in MAINTAINERS file,
> from Denis Efremov.
>
> 2) Counter arguments are swapped in ebtables, from
> Todd Seidelmann.
>
> 3) Missing netlink attribute validation in flow_offload
> extension.
>
> 4) Incorrect alignment in xt_nfacct that breaks 32-bits
> userspace / 64-bits kernels, from Juliana Rodrigueiro.
>
> 5) Missing include guard in nf_conntrack_h323_types.h,
> from Masahiro Yamada.
>
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Pulled, thanks.
^ permalink raw reply
* Re: What to do when a bridge port gets its pvid deleted?
From: Ido Schimmel @ 2019-08-19 20:15 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Florian Fainelli, roopa, nikolay, netdev, Andrew Lunn,
Vivien Didelot, stephen
In-Reply-To: <4756358f-6717-0fbc-3fe8-9f6359583367@gmail.com>
On Mon, Aug 19, 2019 at 08:15:03PM +0300, Vladimir Oltean wrote:
> On 6/28/19 7:45 PM, Florian Fainelli wrote:
> > On 6/28/19 5:37 AM, Vladimir Oltean wrote:
> > > On Fri, 28 Jun 2019 at 15:30, Ido Schimmel <idosch@idosch.org> wrote:
> > > >
> > > > On Tue, Jun 25, 2019 at 11:49:29PM +0300, Vladimir Oltean wrote:
> > > > > A number of DSA drivers (BCM53XX, Microchip KSZ94XX, Mediatek MT7530
> > > > > at the very least), as well as Mellanox Spectrum (I didn't look at all
> > > > > the pure switchdev drivers) try to restore the pvid to a default value
> > > > > on .port_vlan_del.
> > > >
> > > > I don't know about DSA drivers, but that's not what mlxsw is doing. If
> > > > the VLAN that is configured as PVID is deleted from the bridge port, the
> > > > driver instructs the port to discard untagged and prio-tagged packets.
> > > > This is consistent with the bridge driver's behavior.
> > > >
> > > > We do have a flow the "restores" the PVID, but that's when a port is
> > > > unlinked from its bridge master. The PVID we set is 4095 which cannot be
> > > > configured by the 8021q / bridge driver. This is due to the way the
> > > > underlying hardware works. Even if a port is not bridged and used purely
> > > > for routing, packets still do L2 lookup first which sends them directly
> > > > to the router block. If PVID is not configured, untagged packets could
> > > > not be routed. Obviously, at egress we strip this VLAN.
> > > >
> > > > > Sure, the port stops receiving traffic when its pvid is a VLAN ID that
> > > > > is not installed in its hw filter, but as far as the bridge core is
> > > > > concerned, this is to be expected:
> > > > >
> > > > > # bridge vlan add dev swp2 vid 100 pvid untagged
> > > > > # bridge vlan
> > > > > port vlan ids
> > > > > swp5 1 PVID Egress Untagged
> > > > >
> > > > > swp2 1 Egress Untagged
> > > > > 100 PVID Egress Untagged
> > > > >
> > > > > swp3 1 PVID Egress Untagged
> > > > >
> > > > > swp4 1 PVID Egress Untagged
> > > > >
> > > > > br0 1 PVID Egress Untagged
> > > > > # ping 10.0.0.1
> > > > > PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.
> > > > > 64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=0.682 ms
> > > > > 64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=0.299 ms
> > > > > 64 bytes from 10.0.0.1: icmp_seq=3 ttl=64 time=0.251 ms
> > > > > 64 bytes from 10.0.0.1: icmp_seq=4 ttl=64 time=0.324 ms
> > > > > 64 bytes from 10.0.0.1: icmp_seq=5 ttl=64 time=0.257 ms
> > > > > ^C
> > > > > --- 10.0.0.1 ping statistics ---
> > > > > 5 packets transmitted, 5 received, 0% packet loss, time 4188ms
> > > > > rtt min/avg/max/mdev = 0.251/0.362/0.682/0.163 ms
> > > > > # bridge vlan del dev swp2 vid 100
> > > > > # bridge vlan
> > > > > port vlan ids
> > > > > swp5 1 PVID Egress Untagged
> > > > >
> > > > > swp2 1 Egress Untagged
> > > > >
> > > > > swp3 1 PVID Egress Untagged
> > > > >
> > > > > swp4 1 PVID Egress Untagged
> > > > >
> > > > > br0 1 PVID Egress Untagged
> > > > >
> > > > > # ping 10.0.0.1
> > > > > PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.
> > > > > ^C
> > > > > --- 10.0.0.1 ping statistics ---
> > > > > 8 packets transmitted, 0 received, 100% packet loss, time 7267ms
> > > > >
> > > > > What is the consensus here? Is there a reason why the bridge driver
> > > > > doesn't take care of this?
> > > >
> > > > Take care of what? :) Always maintaining a PVID on the bridge port? It's
> > > > completely OK not to have a PVID.
> > > >
> > >
> > > Yes, I didn't think it through during the first email. I came to the
> > > same conclusion in the second one.
> > >
> > > > > Do switchdev drivers have to restore the pvid to always be
> > > > > operational, even if their state becomes inconsistent with the upper
> > > > > dev? Is it just 'nice to have'? What if VID 1 isn't in the hw filter
> > > > > either (perfectly legal)?
> > > >
> > > > Are you saying that DSA drivers always maintain a PVID on the bridge
> > > > port and allow untagged traffic to ingress regardless of the bridge
> > > > driver's configuration? If so, I think this needs to be fixed.
> > >
> > > Well, not at the DSA core level.
> > > But for Microchip:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/microchip/ksz9477.c#n576
> > > For Broadcom:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/b53/b53_common.c#n1376
> > > For Mediatek:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/drivers/net/dsa/mt7530.c#n1196
> > >
> > > There might be others as well.
> >
> > That sounds bogus indeed, and I bet that the two other drivers just
> > followed the b53 driver there. I will have to test this again and come
> > up with a patch eventually.
> >
> > When the port leaves the bridge we do bring it back into a default PVID
> > (which is different than the bridge's default PVID) so that part should
> > be okay.
> >
>
> Adding a few more networking people.
> So my flow is something like this:
> - Boot a board with a DSA switch
> - Bring all interfaces up
> - Enslave all interfaces to br0
> - Enable vlan_filtering on br0
>
> What VIDs should be installed into the ports' hw filters, and what should
> the pvid be at this point?
> Should the switch ports pass any traffic?
> At this point, 'bridge vlan' shows a confusing:
> port vlan ids
> eth0 1 PVID Egress Untagged
>
> swp5 1 PVID Egress Untagged
>
> swp2 1 PVID Egress Untagged
>
> swp3 1 PVID Egress Untagged
>
> swp4 1 PVID Egress Untagged
>
> br0 1 PVID Egress Untagged
> for all ports, but the .port_vlan_add callback is nowhere to be found.
The bridge adds a PVID on the port when it is enslaved to the bridge.
The configuration only takes effect when VLAN filtering is enabled. I'm
looking at dsa_port_vlan_add() and it seems that it does not propagate
the VLAN call when VLAN filtering is disabled. This explains why you
never see the callback.
I assume that if you configure the bridge with VLAN filtering enabled
and then enslave a port, then everything works fine.
mlxsw avoids the situation by forbidding the toggling of VLAN filtering
on the bridge when its ports are enslaved.
>
> Whose responsibility is it for the switch to pass traffic without any
> further 'bridge vlan' command? What is the mechanism through which this
> should work?
>
> What if I do:
> sudo bridge vlan add vid 100 dev swp2 pvid untagged
> echo 0 | sudo tee /sys/class/net/br0/bridge/vlan_filtering
> echo 1 | sudo tee /sys/class/net/br0/bridge/vlan_filtering
> What pvid should there be on swp2 now?
> 'bridge vlan' shows:
> port vlan ids
> eth0 1 PVID Egress Untagged
>
> swp5 1 PVID Egress Untagged
>
> swp2 1 Egress Untagged
> 100 PVID Egress Untagged
>
> swp3 1 PVID Egress Untagged
>
> swp4 1 PVID Egress Untagged
>
> br0 1 PVID Egress Untagged
> If the 'bridge vlan' output is correct, whose responsibility is it to
> restore this pvid?
I suggest to follow mlxsw and avoid this mess. You can support both VLAN
filtering enable / disable without supporting dynamically toggling the
option.
>
> More context: the sja1105 driver is somewhat similar to the mlxsw in that
> VLAN awareness cannot be truly disabled. Arid details aside, in both cases,
> achieving "VLAN-unaware"-like behavior involves manipulating the pvid in
> both cases. But it appears that the bridge core does expect:
> (1) that the driver performs a default VLAN initialization which matches its
> own, without them ever communicating. But because switchdev/DSA drivers
> start off in standalone mode, vlan_filtering=0 comes first, hence the
> non-standard pvid. Through what mechanism is the bridge-expected pvid
> supposed to get restored upon flipping vlan_filtering?
> (2) that toggling VLAN filtering off and on has no other state upon the
> underlying driver than enabling and disabling VLAN awareness. The VLAN hw
> filter table is assumed to be unchanged. Is this a correct assumption?
>
> Thanks,
> -Vladimir
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: dsa: enable and disable all ports
From: David Miller @ 2019-08-19 20:11 UTC (permalink / raw)
To: vivien.didelot; +Cc: netdev, marek.behun, f.fainelli, andrew
In-Reply-To: <20190818173548.19631-1-vivien.didelot@gmail.com>
From: Vivien Didelot <vivien.didelot@gmail.com>
Date: Sun, 18 Aug 2019 13:35:42 -0400
> The DSA stack currently calls the .port_enable and .port_disable switch
> callbacks for slave ports only. However, it is useful to call them for all
> port types. For example this allows some drivers to delay the optimization
> of power consumption after the switch is setup. This can also help reducing
> the setup code of drivers a bit.
>
> The first DSA core patches enable and disable all ports of a switch, regardless
> their type. The last mv88e6xxx patches remove redundant code from the driver
> setup and the said callbacks, now that they handle SERDES power for all ports.
Looks like the bcm_sf2 parts need some adjustments.
^ permalink raw reply
* Re: [PATCH net] tcp: make sure EPOLLOUT wont be missed
From: David Miller @ 2019-08-19 20:08 UTC (permalink / raw)
To: edumazet; +Cc: netdev, soheil, ncardwell, eric.dumazet, jbaron, rutsky
In-Reply-To: <20190817042622.91497-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 16 Aug 2019 21:26:22 -0700
> As Jason Baron explained in commit 790ba4566c1a ("tcp: set SOCK_NOSPACE
> under memory pressure"), it is crucial we properly set SOCK_NOSPACE
> when needed.
>
> However, Jason patch had a bug, because the 'nonblocking' status
> as far as sk_stream_wait_memory() is concerned is governed
> by MSG_DONTWAIT flag passed at sendmsg() time :
>
> long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
>
> So it is very possible that tcp sendmsg() calls sk_stream_wait_memory(),
> and that sk_stream_wait_memory() returns -EAGAIN with SOCK_NOSPACE
> cleared, if sk->sk_sndtimeo has been set to a small (but not zero)
> value.
>
> This patch removes the 'noblock' variable since we must always
> set SOCK_NOSPACE if -EAGAIN is returned.
>
> It also renames the do_nonblock label since we might reach this
> code path even if we were in blocking mode.
>
> Fixes: 790ba4566c1a ("tcp: set SOCK_NOSPACE under memory pressure")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Jason Baron <jbaron@akamai.com>
> Reported-by: Vladimir Rutsky <rutsky@google.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: phy: realtek: support NBase-T MMD EEE registers on RTL8125
From: David Miller @ 2019-08-19 20:05 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <d2669c95-9861-df53-2e37-6ebfde11c4c9@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 16 Aug 2019 21:55:40 +0200
> Add missing EEE-related constants, including the new MMD EEE registers
> for NBase-T / 802.3bz. Based on that emulate the new 802.3bz MMD EEE
> registers for 2.5Gbps EEE on RTL8125.
Series applied.
^ permalink raw reply
* Re: [PATCH net-next] net: flow_offload: convert block_ing_cb_list to regular list type
From: David Miller @ 2019-08-19 20:03 UTC (permalink / raw)
To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri, wenxu, pablo
In-Reply-To: <20190816150654.22106-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Fri, 16 Aug 2019 18:06:54 +0300
> RCU list block_ing_cb_list is protected by rcu read lock in
> flow_block_ing_cmd() and with flow_indr_block_ing_cb_lock mutex in all
> functions that use it. However, flow_block_ing_cmd() needs to call blocking
> functions while iterating block_ing_cb_list which leads to following
> suspicious RCU usage warning:
...
> To fix the described incorrect RCU usage, convert block_ing_cb_list from
> RCU list to regular list and protect it with flow_indr_block_ing_cb_lock
> mutex in flow_block_ing_cmd().
>
> Fixes: 1150ab0f1b33 ("flow_offload: support get multi-subsystem block")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Applied.
^ permalink raw reply
* [PATCH net-next v2 1/6] net: dsa: use a single switch statement for port setup
From: Vivien Didelot @ 2019-08-19 20:00 UTC (permalink / raw)
To: netdev; +Cc: marek.behun, davem, f.fainelli, andrew, Vivien Didelot
In-Reply-To: <20190819200053.21637-1-vivien.didelot@gmail.com>
It is currently difficult to read the different steps involved in the
setup and teardown of ports in the DSA code. Keep it simple with a
single switch statement for each port type: UNUSED, CPU, DSA, or USER.
Also no need to call devlink_port_unregister from within dsa_port_setup
as this step is inconditionally handled by dsa_port_teardown on error.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
net/dsa/dsa2.c | 87 ++++++++++++++++++++++----------------------------
1 file changed, 39 insertions(+), 48 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 3abd173ebacb..405552ac4c08 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -254,88 +254,79 @@ static void dsa_tree_teardown_default_cpu(struct dsa_switch_tree *dst)
static int dsa_port_setup(struct dsa_port *dp)
{
- enum devlink_port_flavour flavour;
struct dsa_switch *ds = dp->ds;
struct dsa_switch_tree *dst = ds->dst;
- int err = 0;
-
- if (dp->type == DSA_PORT_TYPE_UNUSED)
- return 0;
-
- memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
- dp->mac = of_get_mac_address(dp->dn);
-
- switch (dp->type) {
- case DSA_PORT_TYPE_CPU:
- flavour = DEVLINK_PORT_FLAVOUR_CPU;
- break;
- case DSA_PORT_TYPE_DSA:
- flavour = DEVLINK_PORT_FLAVOUR_DSA;
- break;
- case DSA_PORT_TYPE_USER: /* fall-through */
- default:
- flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
- break;
- }
-
- /* dp->index is used now as port_number. However
- * CPU and DSA ports should have separate numbering
- * independent from front panel port numbers.
- */
- devlink_port_attrs_set(&dp->devlink_port, flavour,
- dp->index, false, 0,
- (const char *) &dst->index, sizeof(dst->index));
- err = devlink_port_register(ds->devlink, &dp->devlink_port,
- dp->index);
- if (err)
- return err;
+ const unsigned char *id = (const unsigned char *)&dst->index;
+ const unsigned char len = sizeof(dst->index);
+ struct devlink_port *dlp = &dp->devlink_port;
+ struct devlink *dl = ds->devlink;
+ int err;
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
break;
case DSA_PORT_TYPE_CPU:
+ memset(dlp, 0, sizeof(*dlp));
+ devlink_port_attrs_set(dlp, DEVLINK_PORT_FLAVOUR_CPU,
+ dp->index, false, 0, id, len);
+ err = devlink_port_register(dl, dlp, dp->index);
+ if (err)
+ return err;
+
err = dsa_port_link_register_of(dp);
if (err)
- dev_err(ds->dev, "failed to setup link for port %d.%d\n",
- ds->index, dp->index);
+ return err;
break;
case DSA_PORT_TYPE_DSA:
+ memset(dlp, 0, sizeof(*dlp));
+ devlink_port_attrs_set(dlp, DEVLINK_PORT_FLAVOUR_DSA,
+ dp->index, false, 0, id, len);
+ err = devlink_port_register(dl, dlp, dp->index);
+ if (err)
+ return err;
+
err = dsa_port_link_register_of(dp);
if (err)
- dev_err(ds->dev, "failed to setup link for port %d.%d\n",
- ds->index, dp->index);
+ return err;
break;
case DSA_PORT_TYPE_USER:
+ memset(dlp, 0, sizeof(*dlp));
+ devlink_port_attrs_set(dlp, DEVLINK_PORT_FLAVOUR_PHYSICAL,
+ dp->index, false, 0, id, len);
+ err = devlink_port_register(dl, dlp, dp->index);
+ if (err)
+ return err;
+
+ dp->mac = of_get_mac_address(dp->dn);
err = dsa_slave_create(dp);
if (err)
- dev_err(ds->dev, "failed to create slave for port %d.%d\n",
- ds->index, dp->index);
- else
- devlink_port_type_eth_set(&dp->devlink_port, dp->slave);
+ return err;
+
+ devlink_port_type_eth_set(dlp, dp->slave);
break;
}
- if (err)
- devlink_port_unregister(&dp->devlink_port);
-
- return err;
+ return 0;
}
static void dsa_port_teardown(struct dsa_port *dp)
{
- if (dp->type != DSA_PORT_TYPE_UNUSED)
- devlink_port_unregister(&dp->devlink_port);
+ struct devlink_port *dlp = &dp->devlink_port;
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
break;
case DSA_PORT_TYPE_CPU:
dsa_tag_driver_put(dp->tag_ops);
- /* fall-through */
+ devlink_port_unregister(dlp);
+ dsa_port_link_unregister_of(dp);
+ break;
case DSA_PORT_TYPE_DSA:
+ devlink_port_unregister(dlp);
dsa_port_link_unregister_of(dp);
break;
case DSA_PORT_TYPE_USER:
+ devlink_port_unregister(dlp);
if (dp->slave) {
dsa_slave_destroy(dp->slave);
dp->slave = NULL;
--
2.22.0
^ permalink raw reply related
* [PATCH net-next v2 6/6] net: dsa: mv88e6xxx: wrap SERDES IRQ in power function
From: Vivien Didelot @ 2019-08-19 20:00 UTC (permalink / raw)
To: netdev; +Cc: marek.behun, davem, f.fainelli, andrew, Vivien Didelot
In-Reply-To: <20190819200053.21637-1-vivien.didelot@gmail.com>
Now that mv88e6xxx_serdes_power is only called after driver setup,
we can wrap the SERDES IRQ code directly within it for clarity.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c72b3db75c54..d0bf98c10b2b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2057,10 +2057,26 @@ static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
static int mv88e6xxx_serdes_power(struct mv88e6xxx_chip *chip, int port,
bool on)
{
- if (chip->info->ops->serdes_power)
- return chip->info->ops->serdes_power(chip, port, on);
+ int err;
- return 0;
+ if (!chip->info->ops->serdes_power)
+ return 0;
+
+ if (on) {
+ err = chip->info->ops->serdes_power(chip, port, true);
+ if (err)
+ return err;
+
+ if (chip->info->ops->serdes_irq_setup)
+ err = chip->info->ops->serdes_irq_setup(chip, port);
+ } else {
+ if (chip->info->ops->serdes_irq_free)
+ chip->info->ops->serdes_irq_free(chip, port);
+
+ err = chip->info->ops->serdes_power(chip, port, false);
+ }
+
+ return err;
}
static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)
@@ -2258,12 +2274,7 @@ static int mv88e6xxx_port_enable(struct dsa_switch *ds, int port,
int err;
mv88e6xxx_reg_lock(chip);
-
err = mv88e6xxx_serdes_power(chip, port, true);
-
- if (!err && chip->info->ops->serdes_irq_setup)
- err = chip->info->ops->serdes_irq_setup(chip, port);
-
mv88e6xxx_reg_unlock(chip);
return err;
@@ -2274,13 +2285,8 @@ static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port)
struct mv88e6xxx_chip *chip = ds->priv;
mv88e6xxx_reg_lock(chip);
-
- if (chip->info->ops->serdes_irq_free)
- chip->info->ops->serdes_irq_free(chip, port);
-
if (mv88e6xxx_serdes_power(chip, port, false))
dev_err(chip->dev, "failed to power off SERDES\n");
-
mv88e6xxx_reg_unlock(chip);
}
--
2.22.0
^ permalink raw reply related
* [PATCH net-next v2 5/6] net: dsa: mv88e6xxx: enable SERDES after setup
From: Vivien Didelot @ 2019-08-19 20:00 UTC (permalink / raw)
To: netdev; +Cc: marek.behun, davem, f.fainelli, andrew, Vivien Didelot
In-Reply-To: <20190819200053.21637-1-vivien.didelot@gmail.com>
SERDES is powered on for CPU and DSA ports and powered down for unused
ports at setup time. But now that DSA calls mv88e6xxx_port_enable
and mv88e6xxx_port_disable for all ports, the SERDES power can now
be handled after setup inconditionally for all ports.
Using the port enable and disable callbacks also have the benefit to
handle the SERDES IRQ for non user ports as well.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 35 ++++----------------------------
1 file changed, 4 insertions(+), 31 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 27e1622bb03b..c72b3db75c54 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2151,16 +2151,6 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
if (err)
return err;
- /* Enable the SERDES interface for DSA and CPU ports. Normal
- * ports SERDES are enabled when the port is enabled, thus
- * saving a bit of power.
- */
- if ((dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))) {
- err = mv88e6xxx_serdes_power(chip, port, true);
- if (err)
- return err;
- }
-
/* Port Control 2: don't force a good FCS, set the maximum frame size to
* 10240 bytes, disable 802.1q tags checking, don't discard tagged or
* untagged frames on this port, do a destination address lookup on all
@@ -2267,9 +2257,6 @@ static int mv88e6xxx_port_enable(struct dsa_switch *ds, int port,
struct mv88e6xxx_chip *chip = ds->priv;
int err;
- if (!dsa_is_user_port(ds, port))
- return 0;
-
mv88e6xxx_reg_lock(chip);
err = mv88e6xxx_serdes_power(chip, port, true);
@@ -2286,9 +2273,6 @@ static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port)
{
struct mv88e6xxx_chip *chip = ds->priv;
- if (!dsa_is_user_port(ds, port))
- return;
-
mv88e6xxx_reg_lock(chip);
if (chip->info->ops->serdes_irq_free)
@@ -2461,27 +2445,16 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
/* Setup Switch Port Registers */
for (i = 0; i < mv88e6xxx_num_ports(chip); i++) {
+ if (dsa_is_unused_port(ds, i))
+ continue;
+
/* Prevent the use of an invalid port. */
- if (mv88e6xxx_is_invalid_port(chip, i) &&
- !dsa_is_unused_port(ds, i)) {
+ if (mv88e6xxx_is_invalid_port(chip, i)) {
dev_err(chip->dev, "port %d is invalid\n", i);
err = -EINVAL;
goto unlock;
}
- if (dsa_is_unused_port(ds, i)) {
- err = mv88e6xxx_port_set_state(chip, i,
- BR_STATE_DISABLED);
- if (err)
- goto unlock;
-
- err = mv88e6xxx_serdes_power(chip, i, false);
- if (err)
- goto unlock;
-
- continue;
- }
-
err = mv88e6xxx_setup_port(chip, i);
if (err)
goto unlock;
--
2.22.0
^ permalink raw reply related
* [PATCH net-next v2 4/6] net: dsa: mv88e6xxx: do not change STP state on port disabling
From: Vivien Didelot @ 2019-08-19 20:00 UTC (permalink / raw)
To: netdev; +Cc: marek.behun, davem, f.fainelli, andrew, Vivien Didelot
In-Reply-To: <20190819200053.21637-1-vivien.didelot@gmail.com>
When disabling a port, that is not for the driver to decide what to
do with the STP state. This is already handled by the DSA layer.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 5e557545df6d..27e1622bb03b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2291,9 +2291,6 @@ static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port)
mv88e6xxx_reg_lock(chip);
- if (mv88e6xxx_port_set_state(chip, port, BR_STATE_DISABLED))
- dev_err(chip->dev, "failed to disable port\n");
-
if (chip->info->ops->serdes_irq_free)
chip->info->ops->serdes_irq_free(chip, port);
--
2.22.0
^ permalink raw reply related
* [PATCH net-next v2 3/6] net: dsa: enable and disable all ports
From: Vivien Didelot @ 2019-08-19 20:00 UTC (permalink / raw)
To: netdev; +Cc: marek.behun, davem, f.fainelli, andrew, Vivien Didelot
In-Reply-To: <20190819200053.21637-1-vivien.didelot@gmail.com>
Call the .port_enable and .port_disable functions for all ports,
not only the user ports, so that drivers may optimize the power
consumption of all ports after a successful setup.
Unused ports are now disabled on setup. CPU and DSA ports are now
enabled on setup and disabled on teardown. User ports were already
enabled at slave creation and disabled at slave destruction.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 405552ac4c08..8c4eccb0cfe6 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -264,6 +264,7 @@ static int dsa_port_setup(struct dsa_port *dp)
switch (dp->type) {
case DSA_PORT_TYPE_UNUSED:
+ dsa_port_disable(dp);
break;
case DSA_PORT_TYPE_CPU:
memset(dlp, 0, sizeof(*dlp));
@@ -274,6 +275,10 @@ static int dsa_port_setup(struct dsa_port *dp)
return err;
err = dsa_port_link_register_of(dp);
+ if (err)
+ return err;
+
+ err = dsa_port_enable(dp, NULL);
if (err)
return err;
break;
@@ -286,6 +291,10 @@ static int dsa_port_setup(struct dsa_port *dp)
return err;
err = dsa_port_link_register_of(dp);
+ if (err)
+ return err;
+
+ err = dsa_port_enable(dp, NULL);
if (err)
return err;
break;
@@ -317,11 +326,13 @@ static void dsa_port_teardown(struct dsa_port *dp)
case DSA_PORT_TYPE_UNUSED:
break;
case DSA_PORT_TYPE_CPU:
+ dsa_port_disable(dp);
dsa_tag_driver_put(dp->tag_ops);
devlink_port_unregister(dlp);
dsa_port_link_unregister_of(dp);
break;
case DSA_PORT_TYPE_DSA:
+ dsa_port_disable(dp);
devlink_port_unregister(dlp);
dsa_port_link_unregister_of(dp);
break;
--
2.22.0
^ permalink raw reply related
* [PATCH net-next v2 2/6] net: dsa: do not enable or disable non user ports
From: Vivien Didelot @ 2019-08-19 20:00 UTC (permalink / raw)
To: netdev; +Cc: marek.behun, davem, f.fainelli, andrew, Vivien Didelot
In-Reply-To: <20190819200053.21637-1-vivien.didelot@gmail.com>
The .port_enable and .port_disable operations are currently only
called for user ports, hence assuming they have a slave device. In
preparation for using these operations for other port types as well,
simply guard all implementations against non user ports and return
directly in such case.
Note that bcm_sf2_sw_suspend() currently calls bcm_sf2_port_disable()
(and thus b53_disable_port()) against the user and CPU ports, so do
not guards those functions. They will be called for unused ports in
the future, but that was expected by those drivers anyway.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 7 ++++++-
drivers/net/dsa/bcm_sf2.c | 3 +++
drivers/net/dsa/lan9303-core.c | 6 ++++++
drivers/net/dsa/lantiq_gswip.c | 6 ++++++
drivers/net/dsa/microchip/ksz_common.c | 6 ++++++
drivers/net/dsa/mt7530.c | 6 ++++++
drivers/net/dsa/mv88e6xxx/chip.c | 6 ++++++
7 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 907af62846ba..7d328a5f0161 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -510,10 +510,15 @@ EXPORT_SYMBOL(b53_imp_vlan_setup);
int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
{
struct b53_device *dev = ds->priv;
- unsigned int cpu_port = ds->ports[port].cpu_dp->index;
+ unsigned int cpu_port;
int ret = 0;
u16 pvlan;
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
+ cpu_port = ds->ports[port].cpu_dp->index;
+
if (dev->ops->irq_enable)
ret = dev->ops->irq_enable(dev, port);
if (ret)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 49f99436018a..4f839348011d 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -157,6 +157,9 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
unsigned int i;
u32 reg;
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
/* Clear the memory power down */
reg = core_readl(priv, CORE_MEM_PSM_VDD_CTRL);
reg &= ~P_TXQ_PSM_VDD(port);
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 7a2063e7737a..bbec86b9418e 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1079,6 +1079,9 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
{
struct lan9303 *chip = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
return lan9303_enable_processing_port(chip, port);
}
@@ -1086,6 +1089,9 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port)
{
struct lan9303 *chip = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return;
+
lan9303_disable_processing_port(chip, port);
lan9303_phy_write(ds, chip->phy_addr_base + port, MII_BMCR, BMCR_PDOWN);
}
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 2175ec13bb2c..a69c9b9878b7 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -642,6 +642,9 @@ static int gswip_port_enable(struct dsa_switch *ds, int port,
struct gswip_priv *priv = ds->priv;
int err;
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
if (!dsa_is_cpu_port(ds, port)) {
err = gswip_add_single_port_br(priv, port, true);
if (err)
@@ -678,6 +681,9 @@ static void gswip_port_disable(struct dsa_switch *ds, int port)
{
struct gswip_priv *priv = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return;
+
if (!dsa_is_cpu_port(ds, port)) {
gswip_mdio_mask(priv, GSWIP_MDIO_PHY_LINK_DOWN,
GSWIP_MDIO_PHY_LINK_MASK,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index b45c7b972cec..b0b870f0c252 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -361,6 +361,9 @@ int ksz_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
{
struct ksz_device *dev = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
/* setup slave port */
dev->dev_ops->port_setup(dev, port, false);
if (dev->dev_ops->phy_setup)
@@ -378,6 +381,9 @@ void ksz_disable_port(struct dsa_switch *ds, int port)
{
struct ksz_device *dev = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return;
+
dev->on_ports &= ~(1 << port);
dev->live_ports &= ~(1 << port);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 3181e95586d6..c48e29486b10 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -726,6 +726,9 @@ mt7530_port_enable(struct dsa_switch *ds, int port,
{
struct mt7530_priv *priv = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
mutex_lock(&priv->reg_mutex);
/* Setup the MAC for the user port */
@@ -751,6 +754,9 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
{
struct mt7530_priv *priv = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return;
+
mutex_lock(&priv->reg_mutex);
/* Clear up all port matrix which could be restored in the next
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 9b3ad22a5b98..5e557545df6d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2267,6 +2267,9 @@ static int mv88e6xxx_port_enable(struct dsa_switch *ds, int port,
struct mv88e6xxx_chip *chip = ds->priv;
int err;
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
mv88e6xxx_reg_lock(chip);
err = mv88e6xxx_serdes_power(chip, port, true);
@@ -2283,6 +2286,9 @@ static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port)
{
struct mv88e6xxx_chip *chip = ds->priv;
+ if (!dsa_is_user_port(ds, port))
+ return;
+
mv88e6xxx_reg_lock(chip);
if (mv88e6xxx_port_set_state(chip, port, BR_STATE_DISABLED))
--
2.22.0
^ permalink raw reply related
* [PATCH net-next v2 0/6] net: dsa: enable and disable all ports
From: Vivien Didelot @ 2019-08-19 20:00 UTC (permalink / raw)
To: netdev; +Cc: marek.behun, davem, f.fainelli, andrew, Vivien Didelot
The DSA stack currently calls the .port_enable and .port_disable switch
callbacks for slave ports only. However, it is useful to call them for all
port types. For example this allows some drivers to delay the optimization
of power consumption after the switch is setup. This can also help reducing
the setup code of drivers a bit.
The first DSA core patches enable and disable all ports of a switch, regardless
their type. The last mv88e6xxx patches remove redundant code from the driver
setup and the said callbacks, now that they handle SERDES power for all ports.
Changes in v2: do not guard .port_disable for broadcom switches.
Vivien Didelot (6):
net: dsa: use a single switch statement for port setup
net: dsa: do not enable or disable non user ports
net: dsa: enable and disable all ports
net: dsa: mv88e6xxx: do not change STP state on port disabling
net: dsa: mv88e6xxx: enable SERDES after setup
net: dsa: mv88e6xxx: wrap SERDES IRQ in power function
drivers/net/dsa/b53/b53_common.c | 7 +-
drivers/net/dsa/bcm_sf2.c | 3 +
drivers/net/dsa/lan9303-core.c | 6 ++
drivers/net/dsa/lantiq_gswip.c | 6 ++
drivers/net/dsa/microchip/ksz_common.c | 6 ++
drivers/net/dsa/mt7530.c | 6 ++
drivers/net/dsa/mv88e6xxx/chip.c | 64 ++++++-----------
net/dsa/dsa2.c | 98 +++++++++++++-------------
8 files changed, 106 insertions(+), 90 deletions(-)
--
2.22.0
^ permalink raw reply
* Re: [PATCH net-next 1/1] Add genphy_c45_config_aneg() function to phy-c45.c
From: Heiner Kallweit @ 2019-08-19 19:51 UTC (permalink / raw)
To: Marco Hartmann, Christian Herber, andrew@lunn.ch,
f.fainelli@gmail.com, davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1566237157-9054-2-git-send-email-marco.hartmann@nxp.com>
On 19.08.2019 19:52, Marco Hartmann wrote:
> and call it from phy_config_aneg().
>
Here something went wrong.
> commit 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from
> calling genphy_config_aneg") introduced a check that aborts
> phy_config_aneg() if the phy is a C45 phy.
> This causes phy_state_machine() to call phy_error() so that the phy
> ends up in PHY_HALTED state.
>
> Instead of returning -EOPNOTSUPP, call genphy_c45_config_aneg()
> (analogous to the C22 case) so that the state machine can run
> correctly.
>
> genphy_c45_config_aneg() closely resembles mv3310_config_aneg()
> in drivers/net/phy/marvell10g.c, excluding vendor specific
> configurations for 1000BaseT.
>
> Fixes: 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from
> calling genphy_config_aneg")
>
This tag seems to be the wrong one. This change was done before
genphy_c45_driver was added. Most likely tag should be:
22b56e827093 ("net: phy: replace genphy_10g_driver with genphy_c45_driver")
And because it's a fix applying to previous kernel versions it should
be annotated "net", not "net-next".
> Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
> ---
> drivers/net/phy/phy-c45.c | 26 ++++++++++++++++++++++++++
> drivers/net/phy/phy.c | 2 +-
> include/linux/phy.h | 1 +
> 3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index b9d4145781ca..fa9062fd9122 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -509,6 +509,32 @@ int genphy_c45_read_status(struct phy_device *phydev)
> }
> EXPORT_SYMBOL_GPL(genphy_c45_read_status);
>
> +/**
> + * genphy_c45_config_aneg - restart auto-negotiation or forced setup
> + * @phydev: target phy_device struct
> + *
> + * Description: If auto-negotiation is enabled, we configure the
> + * advertising, and then restart auto-negotiation. If it is not
> + * enabled, then we force a configuration.
> + */
> +int genphy_c45_config_aneg(struct phy_device *phydev)
> +{
> + int ret;
> + bool changed = false;
Reverse xmas tree please.
> [...]
Overall looks good to me. For a single patch you don't have to provide
a cover letter.
^ permalink raw reply
* [PATCH net-next,v2 4/6] net/mlx5: Add HV VHCA infrastructure
From: Haiyang Zhang @ 2019-08-19 19:30 UTC (permalink / raw)
To: sashal@kernel.org, davem@davemloft.net, saeedm@mellanox.com,
leon@kernel.org, eranbe@mellanox.com, lorenzo.pieralisi@arm.com,
bhelgaas@google.com, linux-pci@vger.kernel.org,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
linux-kernel@vger.kernel.org
In-Reply-To: <1566242976-108801-1-git-send-email-haiyangz@microsoft.com>
From: Eran Ben Elisha <eranbe@mellanox.com>
HV VHCA is a layer which provides PF to VF communication channel based on
HyperV PCI config channel. It implements Mellanox's Inter VHCA control
communication protocol. The protocol contains control block in order to
pass messages between the PF and VF drivers, and data blocks in order to
pass actual data.
The infrastructure is agent based. Each agent will be responsible of
contiguous buffer blocks in the VHCA config space. This infrastructure will
bind agents to their blocks, and those agents can only access read/write
the buffer blocks assigned to them. Each agent will provide three
callbacks (control, invalidate, cleanup). Control will be invoked when
block-0 is invalidated with a command that concerns this agent. Invalidate
callback will be invoked if one of the blocks assigned to this agent was
invalidated. Cleanup will be invoked before the agent is being freed in
order to clean all of its open resources or deferred works.
Block-0 serves as the control block. All execution commands from the PF
will be written by the PF over this block. VF will ack on those by
writing on block-0 as well. Its format is described by struct
mlx5_hv_vhca_control_block layout.
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/Makefile | 2 +-
.../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c | 253 +++++++++++++++++++++
.../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h | 102 +++++++++
drivers/net/ethernet/mellanox/mlx5/core/main.c | 7 +
include/linux/mlx5/driver.h | 2 +
5 files changed, 365 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 247295b..fc59a40 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -45,7 +45,7 @@ mlx5_core-$(CONFIG_MLX5_ESWITCH) += eswitch.o eswitch_offloads.o eswitch_offlo
mlx5_core-$(CONFIG_MLX5_MPFS) += lib/mpfs.o
mlx5_core-$(CONFIG_VXLAN) += lib/vxlan.o
mlx5_core-$(CONFIG_PTP_1588_CLOCK) += lib/clock.o
-mlx5_core-$(CONFIG_PCI_HYPERV_INTERFACE) += lib/hv.o
+mlx5_core-$(CONFIG_PCI_HYPERV_INTERFACE) += lib/hv.o lib/hv_vhca.o
#
# Ipoib netdev
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
new file mode 100644
index 0000000..84d1d75
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+// Copyright (c) 2018 Mellanox Technologies
+
+#include <linux/hyperv.h>
+#include "mlx5_core.h"
+#include "lib/hv.h"
+#include "lib/hv_vhca.h"
+
+struct mlx5_hv_vhca {
+ struct mlx5_core_dev *dev;
+ struct workqueue_struct *work_queue;
+ struct mlx5_hv_vhca_agent *agents[MLX5_HV_VHCA_AGENT_MAX];
+ struct mutex agents_lock; /* Protect agents array */
+};
+
+struct mlx5_hv_vhca_work {
+ struct work_struct invalidate_work;
+ struct mlx5_hv_vhca *hv_vhca;
+ u64 block_mask;
+};
+
+struct mlx5_hv_vhca_data_block {
+ u16 sequence;
+ u16 offset;
+ u8 reserved[4];
+ u64 data[15];
+};
+
+struct mlx5_hv_vhca_agent {
+ enum mlx5_hv_vhca_agent_type type;
+ struct mlx5_hv_vhca *hv_vhca;
+ void *priv;
+ u16 seq;
+ void (*control)(struct mlx5_hv_vhca_agent *agent,
+ struct mlx5_hv_vhca_control_block *block);
+ void (*invalidate)(struct mlx5_hv_vhca_agent *agent,
+ u64 block_mask);
+ void (*cleanup)(struct mlx5_hv_vhca_agent *agent);
+};
+
+struct mlx5_hv_vhca *mlx5_hv_vhca_create(struct mlx5_core_dev *dev)
+{
+ struct mlx5_hv_vhca *hv_vhca = NULL;
+
+ hv_vhca = kzalloc(sizeof(*hv_vhca), GFP_KERNEL);
+ if (!hv_vhca)
+ return ERR_PTR(-ENOMEM);
+
+ hv_vhca->work_queue = create_singlethread_workqueue("mlx5_hv_vhca");
+ if (!hv_vhca->work_queue) {
+ kfree(hv_vhca);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ hv_vhca->dev = dev;
+ mutex_init(&hv_vhca->agents_lock);
+
+ return hv_vhca;
+}
+
+void mlx5_hv_vhca_destroy(struct mlx5_hv_vhca *hv_vhca)
+{
+ if (IS_ERR_OR_NULL(hv_vhca))
+ return;
+
+ destroy_workqueue(hv_vhca->work_queue);
+ kfree(hv_vhca);
+}
+
+static void mlx5_hv_vhca_invalidate_work(struct work_struct *work)
+{
+ struct mlx5_hv_vhca_work *hwork;
+ struct mlx5_hv_vhca *hv_vhca;
+ int i;
+
+ hwork = container_of(work, struct mlx5_hv_vhca_work, invalidate_work);
+ hv_vhca = hwork->hv_vhca;
+
+ mutex_lock(&hv_vhca->agents_lock);
+ for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
+ struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
+
+ if (!agent || !agent->invalidate)
+ continue;
+
+ if (!(BIT(agent->type) & hwork->block_mask))
+ continue;
+
+ agent->invalidate(agent, hwork->block_mask);
+ }
+ mutex_unlock(&hv_vhca->agents_lock);
+
+ kfree(hwork);
+}
+
+void mlx5_hv_vhca_invalidate(void *context, u64 block_mask)
+{
+ struct mlx5_hv_vhca *hv_vhca = (struct mlx5_hv_vhca *)context;
+ struct mlx5_hv_vhca_work *work;
+
+ work = kzalloc(sizeof(*work), GFP_ATOMIC);
+ if (!work)
+ return;
+
+ INIT_WORK(&work->invalidate_work, mlx5_hv_vhca_invalidate_work);
+ work->hv_vhca = hv_vhca;
+ work->block_mask = block_mask;
+
+ queue_work(hv_vhca->work_queue, &work->invalidate_work);
+}
+
+int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca)
+{
+ if (IS_ERR_OR_NULL(hv_vhca))
+ return IS_ERR_OR_NULL(hv_vhca);
+
+ return mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
+ mlx5_hv_vhca_invalidate);
+}
+
+void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
+{
+ int i;
+
+ if (IS_ERR_OR_NULL(hv_vhca))
+ return;
+
+ mutex_lock(&hv_vhca->agents_lock);
+ for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++)
+ WARN_ON(hv_vhca->agents[i]);
+
+ mutex_unlock(&hv_vhca->agents_lock);
+
+ mlx5_hv_unregister_invalidate(hv_vhca->dev);
+}
+
+struct mlx5_hv_vhca_agent *
+mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
+ enum mlx5_hv_vhca_agent_type type,
+ void (*control)(struct mlx5_hv_vhca_agent*,
+ struct mlx5_hv_vhca_control_block *block),
+ void (*invalidate)(struct mlx5_hv_vhca_agent*,
+ u64 block_mask),
+ void (*cleaup)(struct mlx5_hv_vhca_agent *agent),
+ void *priv)
+{
+ struct mlx5_hv_vhca_agent *agent;
+
+ if (IS_ERR_OR_NULL(hv_vhca))
+ return ERR_PTR(-ENOMEM);
+
+ if (type >= MLX5_HV_VHCA_AGENT_MAX)
+ return ERR_PTR(-EINVAL);
+
+ mutex_lock(&hv_vhca->agents_lock);
+ if (hv_vhca->agents[type]) {
+ mutex_unlock(&hv_vhca->agents_lock);
+ return ERR_PTR(-EINVAL);
+ }
+ mutex_unlock(&hv_vhca->agents_lock);
+
+ agent = kzalloc(sizeof(*agent), GFP_KERNEL);
+ if (!agent)
+ return ERR_PTR(-ENOMEM);
+
+ agent->type = type;
+ agent->hv_vhca = hv_vhca;
+ agent->priv = priv;
+ agent->control = control;
+ agent->invalidate = invalidate;
+ agent->cleanup = cleaup;
+
+ mutex_lock(&hv_vhca->agents_lock);
+ hv_vhca->agents[type] = agent;
+ mutex_unlock(&hv_vhca->agents_lock);
+
+ return agent;
+}
+
+void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent)
+{
+ struct mlx5_hv_vhca *hv_vhca = agent->hv_vhca;
+
+ mutex_lock(&hv_vhca->agents_lock);
+
+ if (WARN_ON(agent != hv_vhca->agents[agent->type])) {
+ mutex_unlock(&hv_vhca->agents_lock);
+ return;
+ }
+
+ hv_vhca->agents[agent->type] = NULL;
+ mutex_unlock(&hv_vhca->agents_lock);
+
+ if (agent->cleanup)
+ agent->cleanup(agent);
+
+ kfree(agent);
+}
+
+static int mlx5_hv_vhca_data_block_prepare(struct mlx5_hv_vhca_agent *agent,
+ struct mlx5_hv_vhca_data_block *data_block,
+ void *src, int len, int *offset)
+{
+ int bytes = min_t(int, (int)sizeof(data_block->data), len);
+
+ data_block->sequence = agent->seq;
+ data_block->offset = (*offset)++;
+ memcpy(data_block->data, src, bytes);
+
+ return bytes;
+}
+
+static void mlx5_hv_vhca_agent_seq_update(struct mlx5_hv_vhca_agent *agent)
+{
+ agent->seq++;
+}
+
+int mlx5_hv_vhca_agent_write(struct mlx5_hv_vhca_agent *agent,
+ void *buf, int len)
+{
+ int offset = agent->type * HV_CONFIG_BLOCK_SIZE_MAX;
+ int block_offset = 0;
+ int total = 0;
+ int err;
+
+ while (len) {
+ struct mlx5_hv_vhca_data_block data_block = {0};
+ int bytes;
+
+ bytes = mlx5_hv_vhca_data_block_prepare(agent, &data_block,
+ buf + total,
+ len, &block_offset);
+ if (!bytes)
+ return -ENOMEM;
+
+ err = mlx5_hv_write_config(agent->hv_vhca->dev, &data_block,
+ sizeof(data_block), offset);
+ if (err)
+ return err;
+
+ total += bytes;
+ len -= bytes;
+ }
+
+ mlx5_hv_vhca_agent_seq_update(agent);
+
+ return 0;
+}
+
+void *mlx5_hv_vhca_agent_priv(struct mlx5_hv_vhca_agent *agent)
+{
+ return agent->priv;
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
new file mode 100644
index 0000000..cdf1303
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+/* Copyright (c) 2019 Mellanox Technologies. */
+
+#ifndef __LIB_HV_VHCA_H__
+#define __LIB_HV_VHCA_H__
+
+#include "en.h"
+#include "lib/hv.h"
+
+struct mlx5_hv_vhca_agent;
+struct mlx5_hv_vhca;
+struct mlx5_hv_vhca_control_block;
+
+enum mlx5_hv_vhca_agent_type {
+ MLX5_HV_VHCA_AGENT_MAX = 32,
+};
+
+#if IS_ENABLED(CONFIG_PCI_HYPERV_INTERFACE)
+
+struct mlx5_hv_vhca_control_block {
+ u32 capabilities;
+ u32 control;
+ u16 command;
+ u16 command_ack;
+ u16 version;
+ u16 rings;
+ u32 reserved1[28];
+};
+
+struct mlx5_hv_vhca *mlx5_hv_vhca_create(struct mlx5_core_dev *dev);
+void mlx5_hv_vhca_destroy(struct mlx5_hv_vhca *hv_vhca);
+int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca);
+void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca);
+void mlx5_hv_vhca_invalidate(void *context, u64 block_mask);
+
+struct mlx5_hv_vhca_agent *
+mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
+ enum mlx5_hv_vhca_agent_type type,
+ void (*control)(struct mlx5_hv_vhca_agent*,
+ struct mlx5_hv_vhca_control_block *block),
+ void (*invalidate)(struct mlx5_hv_vhca_agent*,
+ u64 block_mask),
+ void (*cleanup)(struct mlx5_hv_vhca_agent *agent),
+ void *context);
+
+void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent);
+int mlx5_hv_vhca_agent_write(struct mlx5_hv_vhca_agent *agent,
+ void *buf, int len);
+void *mlx5_hv_vhca_agent_priv(struct mlx5_hv_vhca_agent *agent);
+
+#else
+
+static inline struct mlx5_hv_vhca *
+mlx5_hv_vhca_create(struct mlx5_core_dev *dev)
+{
+ return NULL;
+}
+
+static inline void mlx5_hv_vhca_destroy(struct mlx5_hv_vhca *hv_vhca)
+{
+}
+
+static inline int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca)
+{
+ return 0;
+}
+
+static inline void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
+{
+}
+
+static inline void mlx5_hv_vhca_invalidate(void *context,
+ u64 block_mask)
+{
+}
+
+static inline struct mlx5_hv_vhca_agent *
+mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
+ enum mlx5_hv_vhca_agent_type type,
+ void (*control)(struct mlx5_hv_vhca_agent*,
+ struct mlx5_hv_vhca_control_block *block),
+ void (*invalidate)(struct mlx5_hv_vhca_agent*,
+ u64 block_mask),
+ void (*cleanup)(struct mlx5_hv_vhca_agent *agent),
+ void *context)
+{
+ return NULL;
+}
+
+static inline void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent)
+{
+}
+
+static inline int
+mlx5_hv_vhca_write_agent(struct mlx5_hv_vhca_agent *agent,
+ void *buf, int len)
+{
+ return 0;
+}
+#endif
+
+#endif /* __LIB_HV_VHCA_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 80437d4..4b74315 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -69,6 +69,7 @@
#include "lib/pci_vsc.h"
#include "diag/fw_tracer.h"
#include "ecpf.h"
+#include "lib/hv_vhca.h"
MODULE_AUTHOR("Eli Cohen <eli@mellanox.com>");
MODULE_DESCRIPTION("Mellanox 5th generation network adapters (ConnectX series) core driver");
@@ -868,6 +869,7 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
}
dev->tracer = mlx5_fw_tracer_create(dev);
+ dev->hv_vhca = mlx5_hv_vhca_create(dev);
return 0;
@@ -897,6 +899,7 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
static void mlx5_cleanup_once(struct mlx5_core_dev *dev)
{
+ mlx5_hv_vhca_destroy(dev->hv_vhca);
mlx5_fw_tracer_destroy(dev->tracer);
mlx5_fpga_cleanup(dev);
mlx5_eswitch_cleanup(dev->priv.eswitch);
@@ -1063,6 +1066,8 @@ static int mlx5_load(struct mlx5_core_dev *dev)
goto err_fw_tracer;
}
+ mlx5_hv_vhca_init(dev->hv_vhca);
+
err = mlx5_fpga_device_start(dev);
if (err) {
mlx5_core_err(dev, "fpga device start failed %d\n", err);
@@ -1118,6 +1123,7 @@ static int mlx5_load(struct mlx5_core_dev *dev)
err_ipsec_start:
mlx5_fpga_device_stop(dev);
err_fpga_start:
+ mlx5_hv_vhca_cleanup(dev->hv_vhca);
mlx5_fw_tracer_cleanup(dev->tracer);
err_fw_tracer:
mlx5_eq_table_destroy(dev);
@@ -1138,6 +1144,7 @@ static void mlx5_unload(struct mlx5_core_dev *dev)
mlx5_accel_ipsec_cleanup(dev);
mlx5_accel_tls_cleanup(dev);
mlx5_fpga_device_stop(dev);
+ mlx5_hv_vhca_cleanup(dev->hv_vhca);
mlx5_fw_tracer_cleanup(dev->tracer);
mlx5_eq_table_destroy(dev);
mlx5_irq_table_destroy(dev);
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 467de34..0e00cbc 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -638,6 +638,7 @@ struct mlx5_clock {
struct mlx5_fw_tracer;
struct mlx5_vxlan;
struct mlx5_geneve;
+struct mlx5_hv_vhca;
struct mlx5_core_dev {
struct device *device;
@@ -685,6 +686,7 @@ struct mlx5_core_dev {
struct mlx5_ib_clock_info *clock_info;
struct mlx5_fw_tracer *tracer;
u32 vsc_addr;
+ struct mlx5_hv_vhca *hv_vhca;
};
struct mlx5_db {
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next,v2 5/6] net/mlx5: Add HV VHCA control agent
From: Haiyang Zhang @ 2019-08-19 19:30 UTC (permalink / raw)
To: sashal@kernel.org, davem@davemloft.net, saeedm@mellanox.com,
leon@kernel.org, eranbe@mellanox.com, lorenzo.pieralisi@arm.com,
bhelgaas@google.com, linux-pci@vger.kernel.org,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
linux-kernel@vger.kernel.org
In-Reply-To: <1566242976-108801-1-git-send-email-haiyangz@microsoft.com>
From: Eran Ben Elisha <eranbe@mellanox.com>
Control agent is responsible over of the control block (ID 0). It should
update the PF via this block about every capability change. In addition,
upon block 0 invalidate, it should activate all other supported agents
with data requests from the PF.
Upon agent create/destroy, the invalidate callback of the control agent
is being called in order to update the PF driver about this change.
The control agent is an integral part of HV VHCA and will be created
and destroy as part of the HV VHCA init/cleanup flow.
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c | 122 ++++++++++++++++++++-
.../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h | 1 +
2 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
index 84d1d75..4047629 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
@@ -109,22 +109,131 @@ void mlx5_hv_vhca_invalidate(void *context, u64 block_mask)
queue_work(hv_vhca->work_queue, &work->invalidate_work);
}
+#define AGENT_MASK(type) (type ? BIT(type - 1) : 0 /* control */)
+
+static void mlx5_hv_vhca_agents_control(struct mlx5_hv_vhca *hv_vhca,
+ struct mlx5_hv_vhca_control_block *block)
+{
+ int i;
+
+ for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
+ struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
+
+ if (!agent || !agent->control)
+ continue;
+
+ if (!(AGENT_MASK(agent->type) & block->control))
+ continue;
+
+ agent->control(agent, block);
+ }
+}
+
+static void mlx5_hv_vhca_capabilities(struct mlx5_hv_vhca *hv_vhca,
+ u32 *capabilities)
+{
+ int i;
+
+ for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
+ struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
+
+ if (agent)
+ *capabilities |= AGENT_MASK(agent->type);
+ }
+}
+
+static void
+mlx5_hv_vhca_control_agent_invalidate(struct mlx5_hv_vhca_agent *agent,
+ u64 block_mask)
+{
+ struct mlx5_hv_vhca *hv_vhca = agent->hv_vhca;
+ struct mlx5_core_dev *dev = hv_vhca->dev;
+ struct mlx5_hv_vhca_control_block *block;
+ u32 capabilities = 0;
+ int err;
+
+ block = kzalloc(sizeof(*block), GFP_KERNEL);
+ if (!block)
+ return;
+
+ err = mlx5_hv_read_config(dev, block, sizeof(*block), 0);
+ if (err)
+ goto free_block;
+
+ mlx5_hv_vhca_capabilities(hv_vhca, &capabilities);
+
+ /* In case no capabilities, send empty block in return */
+ if (!capabilities) {
+ memset(block, 0, sizeof(*block));
+ goto write;
+ }
+
+ if (block->capabilities != capabilities)
+ block->capabilities = capabilities;
+
+ if (block->control & ~capabilities)
+ goto free_block;
+
+ mlx5_hv_vhca_agents_control(hv_vhca, block);
+ block->command_ack = block->command;
+
+write:
+ mlx5_hv_write_config(dev, block, sizeof(*block), 0);
+
+free_block:
+ kfree(block);
+}
+
+static struct mlx5_hv_vhca_agent *
+mlx5_hv_vhca_control_agent_create(struct mlx5_hv_vhca *hv_vhca)
+{
+ return mlx5_hv_vhca_agent_create(hv_vhca, MLX5_HV_VHCA_AGENT_CONTROL,
+ NULL,
+ mlx5_hv_vhca_control_agent_invalidate,
+ NULL, NULL);
+}
+
+static void mlx5_hv_vhca_control_agent_destroy(struct mlx5_hv_vhca_agent *agent)
+{
+ mlx5_hv_vhca_agent_destroy(agent);
+}
+
int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca)
{
+ struct mlx5_hv_vhca_agent *agent;
+ int err;
+
if (IS_ERR_OR_NULL(hv_vhca))
return IS_ERR_OR_NULL(hv_vhca);
- return mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
- mlx5_hv_vhca_invalidate);
+ err = mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
+ mlx5_hv_vhca_invalidate);
+ if (err)
+ return err;
+
+ agent = mlx5_hv_vhca_control_agent_create(hv_vhca);
+ if (IS_ERR_OR_NULL(agent)) {
+ mlx5_hv_unregister_invalidate(hv_vhca->dev);
+ return IS_ERR_OR_NULL(agent);
+ }
+
+ hv_vhca->agents[MLX5_HV_VHCA_AGENT_CONTROL] = agent;
+
+ return 0;
}
void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
{
+ struct mlx5_hv_vhca_agent *agent;
int i;
if (IS_ERR_OR_NULL(hv_vhca))
return;
+ agent = hv_vhca->agents[MLX5_HV_VHCA_AGENT_CONTROL];
+ if (agent)
+ mlx5_hv_vhca_control_agent_destroy(agent);
+
mutex_lock(&hv_vhca->agents_lock);
for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++)
WARN_ON(hv_vhca->agents[i]);
@@ -134,6 +243,11 @@ void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
mlx5_hv_unregister_invalidate(hv_vhca->dev);
}
+static void mlx5_hv_vhca_agents_update(struct mlx5_hv_vhca *hv_vhca)
+{
+ mlx5_hv_vhca_invalidate(hv_vhca, BIT(MLX5_HV_VHCA_AGENT_CONTROL));
+}
+
struct mlx5_hv_vhca_agent *
mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
enum mlx5_hv_vhca_agent_type type,
@@ -174,6 +288,8 @@ struct mlx5_hv_vhca_agent *
hv_vhca->agents[type] = agent;
mutex_unlock(&hv_vhca->agents_lock);
+ mlx5_hv_vhca_agents_update(hv_vhca);
+
return agent;
}
@@ -195,6 +311,8 @@ void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent)
agent->cleanup(agent);
kfree(agent);
+
+ mlx5_hv_vhca_agents_update(hv_vhca);
}
static int mlx5_hv_vhca_data_block_prepare(struct mlx5_hv_vhca_agent *agent,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
index cdf1303..984e7ad 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
@@ -12,6 +12,7 @@
struct mlx5_hv_vhca_control_block;
enum mlx5_hv_vhca_agent_type {
+ MLX5_HV_VHCA_AGENT_CONTROL = 0,
MLX5_HV_VHCA_AGENT_MAX = 32,
};
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next,v2 6/6] net/mlx5e: Add mlx5e HV VHCA stats agent
From: Haiyang Zhang @ 2019-08-19 19:30 UTC (permalink / raw)
To: sashal@kernel.org, davem@davemloft.net, saeedm@mellanox.com,
leon@kernel.org, eranbe@mellanox.com, lorenzo.pieralisi@arm.com,
bhelgaas@google.com, linux-pci@vger.kernel.org,
linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger,
linux-kernel@vger.kernel.org
In-Reply-To: <1566242976-108801-1-git-send-email-haiyangz@microsoft.com>
From: Eran Ben Elisha <eranbe@mellanox.com>
HV VHCA stats agent is responsible on running a preiodic rx/tx
packets/bytes stats update. Currently the supported format is version
MLX5_HV_VHCA_STATS_VERSION. Block ID 1 is dedicated for statistics data
transfer from the VF to the PF.
The reporter fetch the statistics data from all opened channels, fill it
in a buffer and send it to mlx5_hv_vhca_write_agent.
As the stats layer should include some metadata per block (sequence and
offset), the HV VHCA layer shall modify the buffer before actually send it
over block 1.
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/Makefile | 1 +
drivers/net/ethernet/mellanox/mlx5/core/en.h | 13 ++
.../ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c | 162 +++++++++++++++++++++
.../ethernet/mellanox/mlx5/core/en/hv_vhca_stats.h | 25 ++++
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 +
.../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h | 1 +
6 files changed, 205 insertions(+)
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.h
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index fc59a40..a889a38 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -36,6 +36,7 @@ mlx5_core-$(CONFIG_MLX5_CORE_EN_DCB) += en_dcbnl.o en/port_buffer.o
mlx5_core-$(CONFIG_MLX5_ESWITCH) += en_rep.o en_tc.o en/tc_tun.o lib/port_tun.o lag_mp.o \
lib/geneve.o en/tc_tun_vxlan.o en/tc_tun_gre.o \
en/tc_tun_geneve.o
+mlx5_core-$(CONFIG_PCI_HYPERV_INTERFACE) += en/hv_vhca_stats.o
#
# Core extra
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 8fc5107..4a8008b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -54,6 +54,7 @@
#include "mlx5_core.h"
#include "en_stats.h"
#include "en/fs.h"
+#include "lib/hv_vhca.h"
extern const struct net_device_ops mlx5e_netdev_ops;
struct page_pool;
@@ -777,6 +778,15 @@ struct mlx5e_modify_sq_param {
int rl_index;
};
+#if IS_ENABLED(CONFIG_PCI_HYPERV_INTERFACE)
+struct mlx5e_hv_vhca_stats_agent {
+ struct mlx5_hv_vhca_agent *agent;
+ struct delayed_work work;
+ u16 delay;
+ void *buf;
+};
+#endif
+
struct mlx5e_xsk {
/* UMEMs are stored separately from channels, because we don't want to
* lose them when channels are recreated. The kernel also stores UMEMs,
@@ -848,6 +858,9 @@ struct mlx5e_priv {
struct devlink_health_reporter *tx_reporter;
struct devlink_health_reporter *rx_reporter;
struct mlx5e_xsk xsk;
+#if IS_ENABLED(CONFIG_PCI_HYPERV_INTERFACE)
+ struct mlx5e_hv_vhca_stats_agent stats_agent;
+#endif
};
struct mlx5e_profile {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
new file mode 100644
index 0000000..c37b4ac
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+// Copyright (c) 2018 Mellanox Technologies
+
+#include "en.h"
+#include "en/hv_vhca_stats.h"
+#include "lib/hv_vhca.h"
+#include "lib/hv.h"
+
+struct mlx5e_hv_vhca_per_ring_stats {
+ u64 rx_packets;
+ u64 rx_bytes;
+ u64 tx_packets;
+ u64 tx_bytes;
+};
+
+static void
+mlx5e_hv_vhca_fill_ring_stats(struct mlx5e_priv *priv, int ch,
+ struct mlx5e_hv_vhca_per_ring_stats *data)
+{
+ struct mlx5e_channel_stats *stats;
+ int tc;
+
+ stats = &priv->channel_stats[ch];
+ data->rx_packets = stats->rq.packets;
+ data->rx_bytes = stats->rq.bytes;
+
+ for (tc = 0; tc < priv->max_opened_tc; tc++) {
+ data->tx_packets += stats->sq[tc].packets;
+ data->tx_bytes += stats->sq[tc].bytes;
+ }
+}
+
+static void mlx5e_hv_vhca_fill_stats(struct mlx5e_priv *priv, u64 *data,
+ int buf_len)
+{
+ int ch, i = 0;
+
+ for (ch = 0; ch < priv->max_nch; ch++) {
+ u64 *buf = data + i;
+
+ if (WARN_ON_ONCE(buf +
+ sizeof(struct mlx5e_hv_vhca_per_ring_stats) >
+ data + buf_len))
+ return;
+
+ mlx5e_hv_vhca_fill_ring_stats(priv, ch,
+ (struct mlx5e_hv_vhca_per_ring_stats *)buf);
+ i += sizeof(struct mlx5e_hv_vhca_per_ring_stats) / sizeof(u64);
+ }
+}
+
+static int mlx5e_hv_vhca_stats_buf_size(struct mlx5e_priv *priv)
+{
+ return (sizeof(struct mlx5e_hv_vhca_per_ring_stats) *
+ priv->max_nch);
+}
+
+static void mlx5e_hv_vhca_stats_work(struct work_struct *work)
+{
+ struct mlx5e_hv_vhca_stats_agent *sagent;
+ struct mlx5_hv_vhca_agent *agent;
+ struct delayed_work *dwork;
+ struct mlx5e_priv *priv;
+ int buf_len, rc;
+ void *buf;
+
+ dwork = to_delayed_work(work);
+ sagent = container_of(dwork, struct mlx5e_hv_vhca_stats_agent, work);
+ priv = container_of(sagent, struct mlx5e_priv, stats_agent);
+ buf_len = mlx5e_hv_vhca_stats_buf_size(priv);
+ agent = sagent->agent;
+ buf = sagent->buf;
+
+ memset(buf, 0, buf_len);
+ mlx5e_hv_vhca_fill_stats(priv, buf, buf_len);
+
+ rc = mlx5_hv_vhca_agent_write(agent, buf, buf_len);
+ if (rc) {
+ mlx5_core_err(priv->mdev,
+ "%s: Failed to write stats, err = %d\n",
+ __func__, rc);
+ return;
+ }
+
+ if (sagent->delay)
+ queue_delayed_work(priv->wq, &sagent->work, sagent->delay);
+}
+
+enum {
+ MLX5_HV_VHCA_STATS_VERSION = 1,
+ MLX5_HV_VHCA_STATS_UPDATE_ONCE = 0xFFFF,
+};
+
+static void mlx5e_hv_vhca_stats_control(struct mlx5_hv_vhca_agent *agent,
+ struct mlx5_hv_vhca_control_block *block)
+{
+ struct mlx5e_hv_vhca_stats_agent *sagent;
+ struct mlx5e_priv *priv;
+
+ priv = mlx5_hv_vhca_agent_priv(agent);
+ sagent = &priv->stats_agent;
+
+ block->version = MLX5_HV_VHCA_STATS_VERSION;
+ block->rings = priv->max_nch;
+
+ if (!block->command) {
+ cancel_delayed_work_sync(&priv->stats_agent.work);
+ return;
+ }
+
+ sagent->delay = block->command == MLX5_HV_VHCA_STATS_UPDATE_ONCE ? 0 :
+ msecs_to_jiffies(block->command * 100);
+
+ queue_delayed_work(priv->wq, &sagent->work, sagent->delay);
+}
+
+static void mlx5e_hv_vhca_stats_cleanup(struct mlx5_hv_vhca_agent *agent)
+{
+ struct mlx5e_priv *priv = mlx5_hv_vhca_agent_priv(agent);
+
+ cancel_delayed_work_sync(&priv->stats_agent.work);
+}
+
+int mlx5e_hv_vhca_stats_create(struct mlx5e_priv *priv)
+{
+ int buf_len = mlx5e_hv_vhca_stats_buf_size(priv);
+ struct mlx5_hv_vhca_agent *agent;
+
+ priv->stats_agent.buf = kvzalloc(buf_len, GFP_KERNEL);
+ if (!priv->stats_agent.buf)
+ return -ENOMEM;
+
+ agent = mlx5_hv_vhca_agent_create(priv->mdev->hv_vhca,
+ MLX5_HV_VHCA_AGENT_STATS,
+ mlx5e_hv_vhca_stats_control, NULL,
+ mlx5e_hv_vhca_stats_cleanup,
+ priv);
+
+ if (IS_ERR_OR_NULL(agent)) {
+ if (IS_ERR(agent))
+ netdev_warn(priv->netdev,
+ "Failed to create hv vhca stats agent, err = %ld\n",
+ PTR_ERR(agent));
+
+ kfree(priv->stats_agent.buf);
+ return IS_ERR_OR_NULL(agent);
+ }
+
+ priv->stats_agent.agent = agent;
+ INIT_DELAYED_WORK(&priv->stats_agent.work, mlx5e_hv_vhca_stats_work);
+
+ return 0;
+}
+
+void mlx5e_hv_vhca_stats_destroy(struct mlx5e_priv *priv)
+{
+ if (IS_ERR_OR_NULL(priv->stats_agent.agent))
+ return;
+
+ mlx5_hv_vhca_agent_destroy(priv->stats_agent.agent);
+ kfree(priv->stats_agent.buf);
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.h
new file mode 100644
index 0000000..664463f
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+/* Copyright (c) 2019 Mellanox Technologies. */
+
+#ifndef __MLX5_EN_STATS_VHCA_H__
+#define __MLX5_EN_STATS_VHCA_H__
+#include "en.h"
+
+#if IS_ENABLED(CONFIG_PCI_HYPERV_INTERFACE)
+
+int mlx5e_hv_vhca_stats_create(struct mlx5e_priv *priv);
+void mlx5e_hv_vhca_stats_destroy(struct mlx5e_priv *priv);
+
+#else
+
+static inline int mlx5e_hv_vhca_stats_create(struct mlx5e_priv *priv)
+{
+ return 0;
+}
+
+static inline void mlx5e_hv_vhca_stats_destroy(struct mlx5e_priv *priv)
+{
+}
+#endif
+
+#endif /* __MLX5_EN_STATS_VHCA_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 5721d3d..fac8455 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -64,6 +64,7 @@
#include "en/xsk/setup.h"
#include "en/xsk/rx.h"
#include "en/xsk/tx.h"
+#include "en/hv_vhca_stats.h"
bool mlx5e_check_fragmented_striding_rq_cap(struct mlx5_core_dev *mdev)
@@ -5103,6 +5104,7 @@ static void mlx5e_nic_enable(struct mlx5e_priv *priv)
if (mlx5e_monitor_counter_supported(priv))
mlx5e_monitor_counter_init(priv);
+ mlx5e_hv_vhca_stats_create(priv);
if (netdev->reg_state != NETREG_REGISTERED)
return;
#ifdef CONFIG_MLX5_CORE_EN_DCB
@@ -5135,6 +5137,7 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
queue_work(priv->wq, &priv->set_rx_mode_work);
+ mlx5e_hv_vhca_stats_destroy(priv);
if (mlx5e_monitor_counter_supported(priv))
mlx5e_monitor_counter_cleanup(priv);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
index 984e7ad..4bad6a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
@@ -13,6 +13,7 @@
enum mlx5_hv_vhca_agent_type {
MLX5_HV_VHCA_AGENT_CONTROL = 0,
+ MLX5_HV_VHCA_AGENT_STATS = 1,
MLX5_HV_VHCA_AGENT_MAX = 32,
};
--
1.8.3.1
^ permalink raw reply related
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