* Re: loss of connectivity after enabling vlan_filtering
From: vtolkm @ 2019-06-29 23:04 UTC (permalink / raw)
To: netdev; +Cc: Andrew Lunn
In-Reply-To: <20190629224927.GA26554@lunn.ch>
On 30/06/2019 00:49, Andrew Lunn wrote:
> On Sun, Jun 30, 2019 at 12:01:38AM +0200, vtolkm@googlemail.com wrote:
>> * DSA MV88E6060
>> * iproute2 v.5.0.0-2.0
>> * OpenWRT 19.07 with kernel 4.14.131 armv7l
> The mv88e6060 driver is very simple. It has no support for VLANs. It
> does not even have support for offloading bridging between ports to
> the switch.
>
> The data sheet for this device is open. So if you want to hack on the
> driver, you could do.
>
> Andrew
Are you sure? That is a bit confusing after reading
https://lore.kernel.org/patchwork/patch/575746/
^ permalink raw reply
* Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Andrew Lunn @ 2019-06-29 23:06 UTC (permalink / raw)
To: Ido Schimmel
Cc: f.fainelli, vivien.didelot, netdev@vger.kernel.org, Jiri Pirko,
linux@armlinux.org.uk, davem@davemloft.net
In-Reply-To: <20190629153135.GA17143@splinter>
> We have some tests under tools/testing/selftests/net/forwarding/, but
> not so much for MC. However, even if such tests were to be contributed,
> would you be able to run them on your hardware? I remember that in the
> past you complained about the number of required ports.
Hi Ido
Most devices have 4 or 5 ports. Some have more, some less. Given that
the current tests need a second port for every port under test, this
limits you to 2 ports under test. For IGMP snooping and MC in general,
i don't think that is enough. You need one port as the source of the
MC traffic, one port with a host interested in the traffic, and one
port without interest in the traffic. You can then test that traffic
is correctly forwarded and blocked.
The testing we do with DSA makes use of a test host with 4 interfaces,
and connect them one to one to the switch. That gives us 4 ports under
test, and so that gives us many more possibilities for running
interesting tests.
Andrew
^ permalink raw reply
* Re: loss of connectivity after enabling vlan_filtering
From: Andrew Lunn @ 2019-06-29 23:11 UTC (permalink / raw)
To: vtolkm; +Cc: netdev
In-Reply-To: <6226b473-b232-e1d3-40e9-18d118dd82c4@gmail.com>
On Sun, Jun 30, 2019 at 01:04:50AM +0200, vtolkm@googlemail.com wrote:
>
> On 30/06/2019 00:49, Andrew Lunn wrote:
> > On Sun, Jun 30, 2019 at 12:01:38AM +0200, vtolkm@googlemail.com wrote:
> >> * DSA MV88E6060
> >> * iproute2 v.5.0.0-2.0
> >> * OpenWRT 19.07 with kernel 4.14.131 armv7l
> > The mv88e6060 driver is very simple. It has no support for VLANs. It
> > does not even have support for offloading bridging between ports to
> > the switch.
> >
> > The data sheet for this device is open. So if you want to hack on the
> > driver, you could do.
> >
> > Andrew
>
> Are you sure? That is a bit confusing after reading
> https://lore.kernel.org/patchwork/patch/575746/
Quoting that patch:
This commit implements the switchdev operations to add, delete
and dump VLANs for the Marvell 88E6352 and compatible switch
chips.
Vivien added support for the 6352. That uses the mv88e6xxx driver, not
the mv88e6060. And by compatible switches, he meant those in the 6352
family, so 6172 6176 6240 6352 and probably the 6171 6175 6350 6351.
Andrew
^ permalink raw reply
* Re: loss of connectivity after enabling vlan_filtering
From: vtolkm @ 2019-06-29 23:23 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20190629231119.GC26554@lunn.ch>
On 30/06/2019 01:11, Andrew Lunn wrote:
> On Sun, Jun 30, 2019 at 01:04:50AM +0200, vtolkm@googlemail.com wrote:
>> On 30/06/2019 00:49, Andrew Lunn wrote:
>>> On Sun, Jun 30, 2019 at 12:01:38AM +0200, vtolkm@googlemail.com wrote:
>>>> * DSA MV88E6060
>>>> * iproute2 v.5.0.0-2.0
>>>> * OpenWRT 19.07 with kernel 4.14.131 armv7l
>>> The mv88e6060 driver is very simple. It has no support for VLANs. It
>>> does not even have support for offloading bridging between ports to
>>> the switch.
>>>
>>> The data sheet for this device is open. So if you want to hack on the
>>> driver, you could do.
>>>
>>> Andrew
>> Are you sure? That is a bit confusing after reading
>> https://lore.kernel.org/patchwork/patch/575746/
> Quoting that patch:
>
> This commit implements the switchdev operations to add, delete
> and dump VLANs for the Marvell 88E6352 and compatible switch
> chips.
>
> Vivien added support for the 6352. That uses the mv88e6xxx driver, not
> the mv88e6060. And by compatible switches, he meant those in the 6352
> family, so 6172 6176 6240 6352 and probably the 6171 6175 6350 6351.
>
> Andrew
A simple soul might infer that mv88e6xxx includes MV88E6060, at least
that happened to me apparently (being said simpleton).
That may as it be, and pardon my continued ignorance, how is it
explained then that a command as
# bridge v a dev {bridge} self vid {n} untagged pvid
reflects in
# bridge v s
a/o
# bridge mo
?
If the commands are not implemented one would expect them to fail in the
first place or not reflecting a change at all?
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-06-29 23:47 UTC (permalink / raw)
To: Matthew Garrett
Cc: Andy Lutomirski, Stephen Smalley, James Morris, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Network Development, Chun-Yi Lee, Daniel Borkmann, LSM List
In-Reply-To: <CACdnJuuy7-tkj86njAqtdJ3dUMu-2T8a2y8DC3fMKBK0z9J6ag@mail.gmail.com>
On Fri, Jun 28, 2019 at 11:47 AM Matthew Garrett <mjg59@google.com> wrote:
>
> On Thu, Jun 27, 2019 at 4:27 PM Andy Lutomirski <luto@kernel.org> wrote:
> > They're really quite similar in my mind. Certainly some things in the
> > "integrity" category give absolutely trivial control over the kernel
> > (e.g. modules) while others make it quite challenging (ioperm), but
> > the end result is very similar. And quite a few "confidentiality"
> > things genuinely do allow all kernel memory to be read.
> >
> > I agree that finer-grained distinctions could be useful. My concern is
> > that it's a tradeoff, and the other end of the tradeoff is an ABI
> > stability issue. If someone decides down the road that some feature
> > that is currently "integrity" can be split into a narrow "integrity"
> > feature and a "confidentiality" feature then, if the user policy knows
> > about the individual features, there's a risk of breaking people's
> > systems. If we keep the fine-grained control, do we have a clear
> > compatibility story?
>
> My preference right now is to retain the fine-grained aspect of things
> in the internal API, simply because it'll be more annoying to add it
> back later if we want to. I don't want to expose it via the Lockdown
> user facing API for the reasons you've described, but it's not
> impossible that another LSM would find a way to do this reasonably.
> Does it seem reasonable to punt this discussion out to the point where
> another LSM tries to do something with this information, based on the
> implementation they're attempting?
I think I can get behind this, as long as it's clear to LSM authors
that this list is only a little bit stable. I can certainly see the
use for the fine-grained info being available for auditing.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-06-30 0:12 UTC (permalink / raw)
To: Song Liu, linux-security
Cc: Andy Lutomirski, Networking, bpf, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
linux-abi@vger.kernel.org, kees@chromium.org
In-Reply-To: <3C595328-3ABE-4421-9772-8D41094A4F57@fb.com>
On Fri, Jun 28, 2019 at 12:05 PM Song Liu <songliubraving@fb.com> wrote:
>
> Hi Andy,
>
> > On Jun 27, 2019, at 4:40 PM, Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On 6/27/19 1:19 PM, Song Liu wrote:
> >> This patch introduce unprivileged BPF access. The access control is
> >> achieved via device /dev/bpf. Users with write access to /dev/bpf are able
> >> to call sys_bpf().
> >> Two ioctl command are added to /dev/bpf:
> >> The two commands enable/disable permission to call sys_bpf() for current
> >> task. This permission is noted by bpf_permitted in task_struct. This
> >> permission is inherited during clone(CLONE_THREAD).
> >> Helper function bpf_capable() is added to check whether the task has got
> >> permission via /dev/bpf.
> >
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index 0e079b2298f8..79dc4d641cf3 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
> >> env->insn_aux_data[i].orig_idx = i;
> >> env->prog = *prog;
> >> env->ops = bpf_verifier_ops[env->prog->type];
> >> - is_priv = capable(CAP_SYS_ADMIN);
> >> + is_priv = bpf_capable(CAP_SYS_ADMIN);
> >
> > Huh? This isn't a hardening measure -- the "is_priv" verifier mode allows straight-up leaks of private kernel state to user mode.
> >
> > (For that matter, the pending lockdown stuff should possibly consider this a "confidentiality" issue.)
> >
> >
> > I have a bigger issue with this patch, though: it's a really awkward way to pretend to have capabilities. For bpf, it seems like you could make this be a *real* capability without too much pain since there's only one syscall there. Just find a way to pass an fd to /dev/bpf into the syscall. If this means you need a new bpf_with_cap() syscall that takes an extra argument, so be it. The old bpf() syscall can just translate to bpf_with_cap(..., -1).
> >
> > For a while, I've considered a scheme I call "implicit rights". There would be a directory in /dev called /dev/implicit_rights. This would either be part of devtmpfs or a whole new filesystem -- it would *not* be any other filesystem. The contents would be files that can't be read or written and exist only in memory. You create them with a privileged syscall. Certain actions that are sensitive but not at the level of CAP_SYS_ADMIN (use of large-attack-surface bpf stuff, creation of user namespaces, profiling the kernel, etc) could require an "implicit right". When you do them, if you don't have CAP_SYS_ADMIN, the kernel would do a path walk for, say, /dev/implicit_rights/bpf and, if the object exists, can be opened, and actually refers to the "bpf" rights object, then the action is allowed. Otherwise it's denied.
> >
> > This is extensible, and it doesn't require the rather ugly per-task state of whether it's enabled.
> >
> > For things like creation of user namespaces, there's an existing API, and the default is that it works without privilege. Switching it to an implicit right has the benefit of not requiring code changes to programs that already work as non-root.
> >
> > But, for BPF in particular, this type of compatibility issue doesn't exist now. You already can't use most eBPF functionality without privilege. New bpf-using programs meant to run without privilege are *new*, so they can use a new improved API. So, rather than adding this obnoxious ioctl, just make the API explicit, please.
> >
> > Also, please cc: linux-abi next time.
>
> Thanks for your inputs.
>
> I think we need to clarify the use case here. In this case, we are NOT
> thinking about creating new tools for unprivileged users. Instead, we
> would like to use existing tools without root.
I read patch 4, and I interpret it very differently. Patches 2-4 are
creating a new version of libbpf and a new version of bpftool. Given
this, I see no real justification for adding a new in-kernel per-task
state instead of just pushing the complexity into libbpf.
> On the kernel side, we are not planning provides a subset of safe
> features for unprivileged users. The permission here is all-or-nothing.
This may be a showstopper. I think this series needs an extremely
clear explanation of the security implications of providing access to
/dev/bpf. Is it just exposing more attack surface for kernel bugs, or
is it, *by design*, exposing new privileges. Given the is_priv change
that I pointed out upthread, it appears to be the latter, and I'm
wondering how this is a reasonable thing to do.
>
> Introducing bpf_with_cap() syscall means we need teach these tools to
> manage the fd, and use the new API when necessary. This is clearly not
> easy.
How hard can it be? I looked the the libbpf sources, and there are
really very few call sites of sys_bpf(). You could update all of them
with a very small patch.
Also, on a quick survey of kernel/bpf's capable() calls, I feel like
this series may be misguided. If you want to enable unprivileged or
less privileged use of bpf(), how about going through all of the
capable() calls and handling them one-by-one as appropriate. Here's a
random survey:
map_freeze(): It looks like the only reason for a capable() call is
that there isn't a clear permission model right now defining who owns
a map and therefore may freeze it. Could you not use a check along
the lines of capable_wrt_inode_uidgid() instead of capable()?
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
(attr->prog_flags & BPF_F_ANY_ALIGNMENT) &&
!capable(CAP_SYS_ADMIN))
return -EPERM;
I'm not sure why this is needed at all? Is it a DoS mitigation? If
so, couldn't you find a way to acocunt for inefficient unaligned
access similarly to how you account for complexity in general?
if (attr->insn_cnt == 0 ||
attr->insn_cnt > (capable(CAP_SYS_ADMIN) ?
BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
return -E2BIG;
This is similar. I could imagine a cgroup setting that limits bpf
program complexity. This is very, very different type of privilege
than reading kernel memory.
if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
type != BPF_PROG_TYPE_CGROUP_SKB &&
!capable(CAP_SYS_ADMIN))
return -EPERM;
I suspect you could just delete this check or expand the allowable
unprivileged program types after auditing that the other types have
appropriately restrictive verifiers and require appropriate
permissions to *run* the programs.
In bpf_prog_attach():
if (!capable(CAP_NET_ADMIN))
return -EPERM;
This looks like it wants to be ns_capable() after you audit the code
to make sure it's safe enough.
bpf_prog_get_fd_by_id(): I really think you just need a real
permission model. Anyone can create a file on a filesystem, and there
are reasonable policies that allow appropriate users to open existing
files by name without CAP_SYS_ADMIN. Similarly, anyone can create a
key in the kernel keyring subsystem, and there are well-defined rules
under which someone can access an existing key by name. I think you
should come up with a way to handle this for bpf. Adding a bpffs for
this seems like a decent approach.
bpf_prog_get_info_by_id():
if (!capable(CAP_SYS_ADMIN)) {
info.jited_prog_len = 0;
info.xlated_prog_len = 0;
info.nr_jited_ksyms = 0;
info.nr_jited_func_lens = 0;
info.nr_func_info = 0;
info.nr_line_info = 0;
info.nr_jited_line_info = 0;
goto done;
}
It looks like someone decided this information was sensitive if you
query someone else's program. Again, there are sensible ways to
address this.
Finally, in the verifier:
is_priv = capable(CAP_SYS_ADMIN);
That should just be left alone. Arguably you could make a new
capability CAP_BPF_LEAK_KERNEL_ADDRESSES or similar for this.
As it stands, it seems like bpf() was designed under the assumption
that the kind of security model needed to make it work well for
unprivileged or differently-privileged users would be developed later.
But, instead of developing such a security model, this patch is
introducing a whole new Linux "capability" that just disables all
security. From my perspective, this seems like a bad idea.
So, if anyone cares about my opinion, NAK to the whole concept.
Please instead work on fixing this for real, one capable() check at a
time.
^ permalink raw reply
* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Karsten Wiborg @ 2019-06-30 0:14 UTC (permalink / raw)
To: Heiner Kallweit, nic_swsd, romieu; +Cc: netdev
In-Reply-To: <85548ec0-350b-118f-a60c-4be2235d5e4e@gmail.com>
Hi Heiner,
thanks for the speedy reply.
On 6/30/19 12:09 AM, Heiner Kallweit wrote:
> If r8169 (the mainline driver) is running, why do you want to switch
> to r8168 (the Realtek vendor driver)? The latter is not supported by
> the kernel community.
Well I did install r8168 because r8169 is not working.
Didn't even get to see the MAC of the NIC.
>> 2: eno1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group
>> default qlen 1000
>> link/ether 00:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> Seems like the network isn't started.
Jepp, that is the output from the r8169.
Regards,
Karsten
^ permalink raw reply
* Re: [PATCH rdma-next v4 06/17] RDMA/counter: Add "auto" configuration mode support
From: Jason Gunthorpe @ 2019-06-30 0:32 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Majd Dibbiny,
Mark Zhang, Saeed Mahameed, linux-netdev
In-Reply-To: <20190618172625.13432-7-leon@kernel.org>
On Tue, Jun 18, 2019 at 08:26:14PM +0300, Leon Romanovsky wrote:
> +/**
> + * rdma_counter_bind_qp_auto - Check and bind the QP to a counter base on
> + * the auto-mode rule
> + */
> +int rdma_counter_bind_qp_auto(struct ib_qp *qp, u8 port)
> +{
> + struct rdma_port_counter *port_counter;
> + struct ib_device *dev = qp->device;
> + struct rdma_counter *counter;
> + int ret;
> +
> + if (!rdma_is_port_valid(dev, port))
> + return -EINVAL;
> +
> + port_counter = &dev->port_data[port].port_counter;
> + if (port_counter->mode.mode != RDMA_COUNTER_MODE_AUTO)
> + return 0;
> +
> + counter = rdma_get_counter_auto_mode(qp, port);
> + if (counter) {
> + ret = __rdma_counter_bind_qp(counter, qp);
> + if (ret) {
> + rdma_restrack_put(&counter->res);
> + return ret;
> + }
> + kref_get(&counter->kref);
The counter is left in the xarray while the kref is zero, this
kref_get is wrong..
Using two kref like things at the same time is a bad idea, the
'rdma_get_counter_auto_mode' should return the kref held, not the
restrack get. The restrack_del doesn't happen as long as the kref is
positive, so we don't need the retrack thing here..
> + } else {
> + counter = rdma_counter_alloc(dev, port, RDMA_COUNTER_MODE_AUTO);
> + if (!counter)
> + return -ENOMEM;
> +
> + auto_mode_init_counter(counter, qp, port_counter->mode.mask);
> +
> + ret = __rdma_counter_bind_qp(counter, qp);
> + if (ret)
> + goto err_bind;
> +
> + rdma_counter_res_add(counter, qp);
> + if (!rdma_restrack_get(&counter->res)) {
> + ret = -EINVAL;
> + goto err_get;
> + }
and this shouldn't be needed as the kref is inited to 1 by the
rdma_counter_alloc..
> + }
> +
> + return 0;
> +
> +err_get:
> + __rdma_counter_unbind_qp(qp);
> + __rdma_counter_dealloc(counter);
> +err_bind:
> + rdma_counter_free(counter);
> + return ret;
> +}
And then all this error unwind and all the twisty __ functions should
just be a single kref_put and the release should handle everything.
Jason
^ permalink raw reply
* Re: [PATCH net 1/1] net: openvswitch: fix csum updates for MPLS actions
From: Pravin Shelar @ 2019-06-30 0:33 UTC (permalink / raw)
To: John Hurley
Cc: Linux Kernel Network Developers, David S. Miller, Simon Horman,
Jakub Kicinski, oss-drivers
In-Reply-To: <1561642650-1974-1-git-send-email-john.hurley@netronome.com>
On Thu, Jun 27, 2019 at 6:37 AM John Hurley <john.hurley@netronome.com> wrote:
>
> Skbs may have their checksum value populated by HW. If this is a checksum
> calculated over the entire packet then the CHECKSUM_COMPLETE field is
> marked. Changes to the data pointer on the skb throughout the network
> stack still try to maintain this complete csum value if it is required
> through functions such as skb_postpush_rcsum.
>
> The MPLS actions in Open vSwitch modify a CHECKSUM_COMPLETE value when
> changes are made to packet data without a push or a pull. This occurs when
> the ethertype of the MAC header is changed or when MPLS lse fields are
> modified.
>
> The modification is carried out using the csum_partial function to get the
> csum of a buffer and add it into the larger checksum. The buffer is an
> inversion of the data to be removed followed by the new data. Because the
> csum is calculated over 16 bits and these values align with 16 bits, the
> effect is the removal of the old value from the CHECKSUM_COMPLETE and
> addition of the new value.
>
> However, the csum fed into the function and the outcome of the
> calculation are also inverted. This would only make sense if it was the
> new value rather than the old that was inverted in the input buffer.
>
> Fix the issue by removing the bit inverts in the csum_partial calculation.
>
> The bug was verified and the fix tested by comparing the folded value of
> the updated CHECKSUM_COMPLETE value with the folded value of a full
> software checksum calculation (reset skb->csum to 0 and run
> skb_checksum_complete(skb)). Prior to the fix the outcomes differed but
> after they produce the same result.
>
> Fixes: 25cd9ba0abc0 ("openvswitch: Add basic MPLS support to kernel")
> Fixes: bc7cc5999fd3 ("openvswitch: update checksum in {push,pop}_mpls")
> Signed-off-by: John Hurley <john.hurley@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> ---
Thanks for fixing it.
Acked-by: Pravin B Shelar <pshelar@ovn.org>
> net/openvswitch/actions.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 151518d..bd13146 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -166,8 +166,7 @@ static void update_ethertype(struct sk_buff *skb, struct ethhdr *hdr,
> if (skb->ip_summed == CHECKSUM_COMPLETE) {
> __be16 diff[] = { ~(hdr->h_proto), ethertype };
>
> - skb->csum = ~csum_partial((char *)diff, sizeof(diff),
> - ~skb->csum);
> + skb->csum = csum_partial((char *)diff, sizeof(diff), skb->csum);
> }
>
> hdr->h_proto = ethertype;
> @@ -259,8 +258,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
> if (skb->ip_summed == CHECKSUM_COMPLETE) {
> __be32 diff[] = { ~(stack->label_stack_entry), lse };
>
> - skb->csum = ~csum_partial((char *)diff, sizeof(diff),
> - ~skb->csum);
> + skb->csum = csum_partial((char *)diff, sizeof(diff), skb->csum);
> }
>
> stack->label_stack_entry = lse;
> --
> 2.7.4
>
^ permalink raw reply
* Re: loss of connectivity after enabling vlan_filtering
From: vtolkm @ 2019-06-30 0:37 UTC (permalink / raw)
To: netdev; +Cc: Andrew Lunn
In-Reply-To: <53bd8ffc-1c0a-334d-67d5-3a74b76670e8@gmail.com>
On 30/06/2019 01:23, vtolkm@gmail.com wrote:
> On 30/06/2019 01:11, Andrew Lunn wrote:
>> On Sun, Jun 30, 2019 at 01:04:50AM +0200, vtolkm@googlemail.com wrote:
>>> On 30/06/2019 00:49, Andrew Lunn wrote:
>>>> On Sun, Jun 30, 2019 at 12:01:38AM +0200, vtolkm@googlemail.com wrote:
>>>>> * DSA MV88E6060
>>>>> * iproute2 v.5.0.0-2.0
>>>>> * OpenWRT 19.07 with kernel 4.14.131 armv7l
>>>> The mv88e6060 driver is very simple. It has no support for VLANs. It
>>>> does not even have support for offloading bridging between ports to
>>>> the switch.
>>>>
>>>> The data sheet for this device is open. So if you want to hack on the
>>>> driver, you could do.
>>>>
>>>> Andrew
>>> Are you sure? That is a bit confusing after reading
>>> https://lore.kernel.org/patchwork/patch/575746/
>> Quoting that patch:
>>
>> This commit implements the switchdev operations to add, delete
>> and dump VLANs for the Marvell 88E6352 and compatible switch
>> chips.
>>
>> Vivien added support for the 6352. That uses the mv88e6xxx driver, not
>> the mv88e6060. And by compatible switches, he meant those in the 6352
>> family, so 6172 6176 6240 6352 and probably the 6171 6175 6350 6351.
>>
>> Andrew
> A simple soul might infer that mv88e6xxx includes MV88E6060, at least
> that happened to me apparently (being said simpleton).
> That may as it be, and pardon my continued ignorance, how is it
> explained then that a command as
>
> # bridge v a dev {bridge} self vid {n} untagged pvid
>
> reflects in
>
> # bridge v s
> a/o
> # bridge mo
>
> ?
>
> If the commands are not implemented one would expect them to fail in the
> first place or not reflecting a change at all?
>
>
As stated in the initial message - kernel conf
CONFIG_NET_DSA_MV88E6060=y
CONFIG_NET_DSA_MV88E6XXX=y
CONFIG_NET_DSA_MV88E6XXX_GLOBAL2=y
^ permalink raw reply
* Re: [PATCH rdma-next v4 06/17] RDMA/counter: Add "auto" configuration mode support
From: Jason Gunthorpe @ 2019-06-30 0:40 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Majd Dibbiny,
Mark Zhang, Saeed Mahameed, linux-netdev
In-Reply-To: <20190618172625.13432-7-leon@kernel.org>
On Tue, Jun 18, 2019 at 08:26:14PM +0300, Leon Romanovsky wrote:
> +static void __rdma_counter_dealloc(struct rdma_counter *counter)
> +{
> + mutex_lock(&counter->lock);
> + counter->device->ops.counter_dealloc(counter);
> + mutex_unlock(&counter->lock);
> +}
Does this lock do anything? The kref is 0 at this point, so no other
thread can have a pointer to this lock.
> +
> +static void rdma_counter_dealloc(struct rdma_counter *counter)
> +{
> + if (!counter)
> + return;
Counter is never NULL.
Jason
^ permalink raw reply
* Re: loss of connectivity after enabling vlan_filtering
From: vtolkm @ 2019-06-30 1:13 UTC (permalink / raw)
To: netdev; +Cc: Andrew Lunn
In-Reply-To: <aa021a99-accd-75ac-47a4-2c11aa791d3c@gmail.com>
On 30/06/2019 02:37, vtolkm@gmail.com wrote:
> On 30/06/2019 01:23, vtolkm@gmail.com wrote:
>> On 30/06/2019 01:11, Andrew Lunn wrote:
>>> On Sun, Jun 30, 2019 at 01:04:50AM +0200, vtolkm@googlemail.com wrote:
>>>> On 30/06/2019 00:49, Andrew Lunn wrote:
>>>>> On Sun, Jun 30, 2019 at 12:01:38AM +0200, vtolkm@googlemail.com wrote:
>>>>>> * DSA MV88E6060
>>>>>> * iproute2 v.5.0.0-2.0
>>>>>> * OpenWRT 19.07 with kernel 4.14.131 armv7l
>>>>> The mv88e6060 driver is very simple. It has no support for VLANs. It
>>>>> does not even have support for offloading bridging between ports to
>>>>> the switch.
>>>>>
>>>>> The data sheet for this device is open. So if you want to hack on the
>>>>> driver, you could do.
>>>>>
>>>>> Andrew
>>>> Are you sure? That is a bit confusing after reading
>>>> https://lore.kernel.org/patchwork/patch/575746/
>>> Quoting that patch:
>>>
>>> This commit implements the switchdev operations to add, delete
>>> and dump VLANs for the Marvell 88E6352 and compatible switch
>>> chips.
>>>
>>> Vivien added support for the 6352. That uses the mv88e6xxx driver, not
>>> the mv88e6060. And by compatible switches, he meant those in the 6352
>>> family, so 6172 6176 6240 6352 and probably the 6171 6175 6350 6351.
>>>
>>> Andrew
>> A simple soul might infer that mv88e6xxx includes MV88E6060, at least
>> that happened to me apparently (being said simpleton).
>> That may as it be, and pardon my continued ignorance, how is it
>> explained then that a command as
>>
>> # bridge v a dev {bridge} self vid {n} untagged pvid
>>
>> reflects in
>>
>> # bridge v s
>> a/o
>> # bridge mo
>>
>> ?
>>
>> If the commands are not implemented one would expect them to fail in the
>> first place or not reflecting a change at all?
>>
>>
> As stated in the initial message - kernel conf
>
> CONFIG_NET_DSA_MV88E6060=y
> CONFIG_NET_DSA_MV88E6XXX=y
> CONFIG_NET_DSA_MV88E6XXX_GLOBAL2=y
>
Just figured that it is a Marvell 88E6176-TFJ2. Thus that cannot be the
cause, also considering the that the commands are executing and changes
being reflected.
However, the loss of access to the node is a mystery.
Apparently the filter is doing its job as in isolating access to the
bridge if connecting though an enslaved device.
And yet the bridge is still fully accessible from other devices that or
not enslaved by that bridge. As if the filter is inverted.
^ permalink raw reply
* Re: [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained
From: Masahiro Yamada @ 2019-06-30 1:27 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Linux Kbuild mailing list, Jani Nikula, Song Liu,
Alexei Starovoitov, Networking, Yonghong Song,
Linux Kernel Mailing List, linux-riscv, Michal Marek,
Martin KaFai Lau, Palmer Dabbelt, bpf, Daniel Borkmann, Albert Ou
In-Reply-To: <20190628154349.GA12826@ravnborg.org>
Hi Sam,
On Sat, Jun 29, 2019 at 12:44 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Masahiro.
>
> On Fri, Jun 28, 2019 at 01:38:59AM +0900, Masahiro Yamada wrote:
> > Multiple people have suggested compile-testing UAPI headers to ensure
> > they can be really included from user-space. "make headers_check" is
> > obviously not enough to catch bugs, and we often leak references to
> > kernel-space definition to user-space.
> >
> > Use the new header-test-y syntax to implement it. Please note exported
> > headers are compile-tested with a completely different set of compiler
> > flags. The header search path is set to $(objtree)/usr/include since
> > exported headers should not include unexported ones.
>
> This patchset introduce a new set of tests for uapi headers.
> Can we somehow consolidate so we have only one way to verify the uapi
> headers?
> It can be confusing for users that they see errors from testing the
> uapi headers during normal build and a new class or error if they
> run a "make headers_check" sometimes later.
Good point. I had also noticed some over-wrap
between this feature and scripts/headers_check.pl
For example, check_include will be unneeded.
I expect "make headers_check" will be deprecated
sooner or later.
> This can be a next step to consolidate this.
> With the suggestions below considered you can add my:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>
> >
> > We use -std=gnu89 for the kernel space since the kernel code highly
> > depends on GNU extensions. On the other hand, UAPI headers should be
> > written in more standardized C, so they are compiled with -std=c90.
> > This will emit errors if C++ style comments, the keyword 'inline', etc.
> > are used. Please use C style comments (/* ... */), '__inline__', etc.
> > in UAPI headers.
> >
> > There is additional compiler requirement to enable this test because
> > many of UAPI headers include <stdlib.h>, <sys/ioctl.h>, <sys/time.h>,
> > etc. directly or indirectly. You cannot use kernel.org pre-built
> > toolchains [1] since they lack <stdlib.h>.
> >
> > I added scripts/cc-system-headers.sh to check the system header
> > availability, which CONFIG_UAPI_HEADER_TEST depends on.
> >
> > For now, a lot of headers need to be excluded because they cannot
> > be compiled standalone, but this is a good start point.
> >
> > [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/index.html
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> > Changes in v3: None
> > Changes in v2:
> > - Add CONFIG_CPU_{BIG,LITTLE}_ENDIAN guard to avoid build error
> > - Use 'header-test-' instead of 'no-header-test'
> > - Avoid weird 'find' warning when cleaning
> >
> > Makefile | 2 +-
> > init/Kconfig | 11 +++
> > scripts/cc-system-headers.sh | 8 +++
> > usr/.gitignore | 1 -
> > usr/Makefile | 2 +
> > usr/include/.gitignore | 3 +
> > usr/include/Makefile | 134 +++++++++++++++++++++++++++++++++++
> > 7 files changed, 159 insertions(+), 2 deletions(-)
> > create mode 100755 scripts/cc-system-headers.sh
> > create mode 100644 usr/include/.gitignore
> > create mode 100644 usr/include/Makefile
> >
> > diff --git a/Makefile b/Makefile
> > index 1f35aca4fe05..f23516980796 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1363,7 +1363,7 @@ CLEAN_DIRS += $(MODVERDIR) include/ksym
> > CLEAN_FILES += modules.builtin.modinfo
> >
> > # Directories & files removed with 'make mrproper'
> > -MRPROPER_DIRS += include/config usr/include include/generated \
> > +MRPROPER_DIRS += include/config include/generated \
> > arch/$(SRCARCH)/include/generated .tmp_objdiff
> > MRPROPER_FILES += .config .config.old .version \
> > Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \
> > diff --git a/init/Kconfig b/init/Kconfig
> > index df5bba27e3fe..667d68e1cdf4 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -105,6 +105,17 @@ config HEADER_TEST
> > If you are a developer or tester and want to ensure the requested
> > headers are self-contained, say Y here. Otherwise, choose N.
> >
> > +config UAPI_HEADER_TEST
> > + bool "Compile test UAPI headers"
> > + depends on HEADER_TEST && HEADERS_INSTALL
> > + depends on $(success,$(srctree)/scripts/cc-system-headers.sh $(CC))
> > + help
> > + Compile test headers exported to user-space to ensure they are
> > + self-contained, i.e. compilable as standalone units.
> > +
> > + If you are a developer or tester and want to ensure the exported
> > + headers are self-contained, say Y here. Otherwise, choose N.
> > +
> > config LOCALVERSION
> > string "Local version - append to kernel release"
> > help
> > diff --git a/scripts/cc-system-headers.sh b/scripts/cc-system-headers.sh
> > new file mode 100755
> > index 000000000000..1b3db369828c
> > --- /dev/null
> > +++ b/scripts/cc-system-headers.sh
> > @@ -0,0 +1,8 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +cat << "END" | $@ -E -x c - -o /dev/null >/dev/null 2>&1
> > +#include <stdlib.h>
> > +#include <sys/ioctl.h>
> > +#include <sys/time.h>
> > +END
>
> Add comment to this file that explains that it is used to verify that the
> toolchain provides the minimal set of header-files required by uapi
> headers.
Will do.
> > diff --git a/usr/.gitignore b/usr/.gitignore
> > index 8e48117a3f3d..be5eae1df7eb 100644
> > --- a/usr/.gitignore
> > +++ b/usr/.gitignore
> > @@ -7,4 +7,3 @@ initramfs_data.cpio.gz
> > initramfs_data.cpio.bz2
> > initramfs_data.cpio.lzma
> > initramfs_list
> > -include
> > diff --git a/usr/Makefile b/usr/Makefile
> > index 4a70ae43c9cb..6a89eb019275 100644
> > --- a/usr/Makefile
> > +++ b/usr/Makefile
> > @@ -56,3 +56,5 @@ $(deps_initramfs): klibcdirs
> > $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs
> > $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y)
> > $(call if_changed,initfs)
> > +
> > +subdir-$(CONFIG_UAPI_HEADER_TEST) += include
> > diff --git a/usr/include/.gitignore b/usr/include/.gitignore
> > new file mode 100644
> > index 000000000000..a0991ff4402b
> > --- /dev/null
> > +++ b/usr/include/.gitignore
> > @@ -0,0 +1,3 @@
> > +*
> > +!.gitignore
> > +!Makefile
> > diff --git a/usr/include/Makefile b/usr/include/Makefile
> > new file mode 100644
> > index 000000000000..58ce96fa1701
> > --- /dev/null
> > +++ b/usr/include/Makefile
> > @@ -0,0 +1,134 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +# Unlike the kernel space, uapi headers are written in standard C.
> > +# - Forbid C++ style comments
> > +# - Use '__inline__', '__asm__' instead of 'inline', 'asm'
> > +#
> > +# -std=c90 (equivalent to -ansi) catches the violation of those.
> > +# We cannot go as far as adding -Wpedantic since it emits too many warnings.
> > +#
> > +# REVISIT: re-consider the proper set of compiler flags for uapi compile-test.
> > +
> > +UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration
> > +
> > +override c_flags = $(UAPI_CFLAGS) -Wp,-MD,$(depfile) -I$(objtree)/usr/include
> > +
> > +# The following are excluded for now because they fail to build.
> > +# The cause of errors are mostly missing include directives.
> > +# Check one by one, and send a patch to each subsystem.
> > +#
> > +# Do not add a new header to the blacklist without legitimate reason.
> > +# Please consider to fix the header first.
>
> Maybe add comment that the alphabetical sort by filename must be preserved.
> Not too relevant, as we hopefully do not see files being added.
>
> > +header-test- += asm/ipcbuf.h
> > +header-test- += asm/msgbuf.h
> Consider same syntax like in include/Makefile where you use
> header-test-<tab><tab>...+= file
>
> Then the alignment looks betters.
Probably I can do that, but this is not so important
because our ultimate goal is (almost) no blacklist.
> > +
> > +# The rest are compile-tested
> > +header-test-y += $(filter-out $(header-test-), \
> > + $(patsubst $(obj)/%,%, $(wildcard \
> > + $(addprefix $(obj)/, *.h */*.h */*/*.h */*/*/*.h))))
> Could you use header-test-pattern-y here?
header-test-pattern-y does not work here
because it only matches to $(srctree)/$(src)/*
All headers under usr/include/
are generated one, and located in $(objtree)/$(obj)/.
Thanks.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained
From: Masahiro Yamada @ 2019-06-30 1:35 UTC (permalink / raw)
To: Linux Kbuild mailing list
Cc: Song Liu, Michal Marek, bpf, Daniel Borkmann, Jani Nikula,
Networking, Palmer Dabbelt, Alexei Starovoitov,
Linux Kernel Mailing List, Albert Ou, Yonghong Song, linux-riscv,
Sam Ravnborg, Martin KaFai Lau
In-Reply-To: <20190627163903.28398-2-yamada.masahiro@socionext.com>
On Fri, Jun 28, 2019 at 1:40 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Multiple people have suggested compile-testing UAPI headers to ensure
> they can be really included from user-space. "make headers_check" is
> obviously not enough to catch bugs, and we often leak references to
> kernel-space definition to user-space.
>
> Use the new header-test-y syntax to implement it. Please note exported
> headers are compile-tested with a completely different set of compiler
> flags. The header search path is set to $(objtree)/usr/include since
> exported headers should not include unexported ones.
>
> We use -std=gnu89 for the kernel space since the kernel code highly
> depends on GNU extensions. On the other hand, UAPI headers should be
> written in more standardized C, so they are compiled with -std=c90.
> This will emit errors if C++ style comments, the keyword 'inline', etc.
> are used. Please use C style comments (/* ... */), '__inline__', etc.
> in UAPI headers.
>
> There is additional compiler requirement to enable this test because
> many of UAPI headers include <stdlib.h>, <sys/ioctl.h>, <sys/time.h>,
> etc. directly or indirectly. You cannot use kernel.org pre-built
> toolchains [1] since they lack <stdlib.h>.
>
> I added scripts/cc-system-headers.sh to check the system header
> availability, which CONFIG_UAPI_HEADER_TEST depends on.
Perhaps, we could use scripts/cc-can-link.sh for this purpose.
The intention is slightly different, but a compiler to link
user-space programs must provide necessary standard headers.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Yonghong Song @ 2019-06-30 5:52 UTC (permalink / raw)
To: Stanislav Fomichev, netdev@vger.kernel.org, bpf@vger.kernel.org
Cc: davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net,
Andrii Nakryiko, kernel test robot
In-Reply-To: <20190628231049.22149-1-sdf@google.com>
On 6/28/19 4:10 PM, Stanislav Fomichev wrote:
> Since commit cd17d7770578 ("bpf/tools: sync bpf.h") clang decided
> that it can do a single u64 store into user_ip6[2] instead of two
> separate u32 ones:
>
> # 17: (18) r2 = 0x100000000000000
> # ; ctx->user_ip6[2] = bpf_htonl(DST_REWRITE_IP6_2);
> # 19: (7b) *(u64 *)(r1 +16) = r2
> # invalid bpf_context access off=16 size=8
>
> From the compiler point of view it does look like a correct thing
> to do, so let's support it on the kernel side.
>
> Credit to Andrii Nakryiko for a proper implementation of
> bpf_ctx_wide_store_ok.
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Fixes: cd17d7770578 ("bpf/tools: sync bpf.h")
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
The change looks good to me with the following nits:
1. could you add a cover letter for the patch set?
typically if the number of patches is more than one,
it would be a good practice with a cover letter.
See bpf_devel_QA.rst .
2. with this change, the comments in uapi bpf.h
are not accurate any more.
__u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
* Stored in network byte order.
*/
__u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
* Stored in network byte order.
*/
now for stores, aligned 8-byte write is permitted.
could you update this as well?
From the typical usage pattern, I did not see a need
for 8-tye read of user_ip6 and msg_src_ip6 yet. So let
us just deal with write for now.
With the above two nits,
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> include/linux/filter.h | 6 ++++++
> net/core/filter.c | 22 ++++++++++++++--------
> 2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 340f7d648974..3901007e36f1 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -746,6 +746,12 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
> return size <= size_default && (size & (size - 1)) == 0;
> }
>
> +#define bpf_ctx_wide_store_ok(off, size, type, field) \
> + (size == sizeof(__u64) && \
> + off >= offsetof(type, field) && \
> + off + sizeof(__u64) <= offsetofend(type, field) && \
> + off % sizeof(__u64) == 0)
> +
> #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
>
> static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> diff --git a/net/core/filter.c b/net/core/filter.c
> index dc8534be12fc..5d33f2146dab 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6849,6 +6849,16 @@ static bool sock_addr_is_valid_access(int off, int size,
> if (!bpf_ctx_narrow_access_ok(off, size, size_default))
> return false;
> } else {
> + if (bpf_ctx_wide_store_ok(off, size,
> + struct bpf_sock_addr,
> + user_ip6))
> + return true;
> +
> + if (bpf_ctx_wide_store_ok(off, size,
> + struct bpf_sock_addr,
> + msg_src_ip6))
> + return true;
> +
> if (size != size_default)
> return false;
> }
> @@ -7689,9 +7699,6 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> /* SOCK_ADDR_STORE_NESTED_FIELD_OFF() has semantic similar to
> * SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF() but for store operation.
> *
> - * It doesn't support SIZE argument though since narrow stores are not
> - * supported for now.
> - *
> * In addition it uses Temporary Field TF (member of struct S) as the 3rd
> * "register" since two registers available in convert_ctx_access are not
> * enough: we can't override neither SRC, since it contains value to store, nor
> @@ -7699,7 +7706,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> * instructions. But we need a temporary place to save pointer to nested
> * structure whose field we want to store to.
> */
> -#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, TF) \
> +#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, OFF, TF) \
> do { \
> int tmp_reg = BPF_REG_9; \
> if (si->src_reg == tmp_reg || si->dst_reg == tmp_reg) \
> @@ -7710,8 +7717,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> offsetof(S, TF)); \
> *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(S, F), tmp_reg, \
> si->dst_reg, offsetof(S, F)); \
> - *insn++ = BPF_STX_MEM( \
> - BPF_FIELD_SIZEOF(NS, NF), tmp_reg, si->src_reg, \
> + *insn++ = BPF_STX_MEM(SIZE, tmp_reg, si->src_reg, \
> bpf_target_off(NS, NF, FIELD_SIZEOF(NS, NF), \
> target_size) \
> + OFF); \
> @@ -7723,8 +7729,8 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> TF) \
> do { \
> if (type == BPF_WRITE) { \
> - SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, \
> - TF); \
> + SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, \
> + OFF, TF); \
> } else { \
> SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF( \
> S, NS, F, NF, SIZE, OFF); \
>
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] selftests/bpf: add verifier tests for wide stores
From: Yonghong Song @ 2019-06-30 6:01 UTC (permalink / raw)
To: Stanislav Fomichev, netdev@vger.kernel.org, bpf@vger.kernel.org
Cc: davem@davemloft.net, ast@kernel.org, daniel@iogearbox.net,
Andrii Nakryiko
In-Reply-To: <20190628231049.22149-2-sdf@google.com>
On 6/28/19 4:10 PM, Stanislav Fomichev wrote:
> Make sure that wide stores are allowed at proper (aligned) addresses.
> Note that user_ip6 is naturally aligned on 8-byte boundary, so
> correct addresses are user_ip6[0] and user_ip6[2]. msg_src_ip6 is,
> however, aligned on a 4-byte bondary, so only msg_src_ip6[1]
> can be wide-stored.
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 17 ++++++--
> .../selftests/bpf/verifier/wide_store.c | 40 +++++++++++++++++++
> 2 files changed, 54 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/wide_store.c
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index c5514daf8865..b0773291012a 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -105,6 +105,7 @@ struct bpf_test {
> __u64 data64[TEST_DATA_LEN / 8];
> };
> } retvals[MAX_TEST_RUNS];
> + enum bpf_attach_type expected_attach_type;
> };
>
> /* Note we want this to be 64 bit aligned so that the end of our array is
> @@ -850,6 +851,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> int fd_prog, expected_ret, alignment_prevented_execution;
> int prog_len, prog_type = test->prog_type;
> struct bpf_insn *prog = test->insns;
> + struct bpf_load_program_attr attr;
> int run_errs, run_successes;
> int map_fds[MAX_NR_MAPS];
> const char *expected_err;
> @@ -881,8 +883,17 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> pflags |= BPF_F_STRICT_ALIGNMENT;
> if (test->flags & F_NEEDS_EFFICIENT_UNALIGNED_ACCESS)
> pflags |= BPF_F_ANY_ALIGNMENT;
> - fd_prog = bpf_verify_program(prog_type, prog, prog_len, pflags,
> - "GPL", 0, bpf_vlog, sizeof(bpf_vlog), 4);
> +
> + memset(&attr, 0, sizeof(attr));
> + attr.prog_type = prog_type;
> + attr.expected_attach_type = test->expected_attach_type;
> + attr.insns = prog;
> + attr.insns_cnt = prog_len;
> + attr.license = "GPL";
> + attr.log_level = 4;
> + attr.prog_flags = pflags;
> +
> + fd_prog = bpf_load_program_xattr(&attr, bpf_vlog, sizeof(bpf_vlog));
> if (fd_prog < 0 && !bpf_probe_prog_type(prog_type, 0)) {
> printf("SKIP (unsupported program type %d)\n", prog_type);
> skips++;
> @@ -912,7 +923,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> printf("FAIL\nUnexpected success to load!\n");
> goto fail_log;
> }
> - if (!strstr(bpf_vlog, expected_err)) {
> + if (!expected_err || !strstr(bpf_vlog, expected_err)) {
> printf("FAIL\nUnexpected error message!\n\tEXP: %s\n\tRES: %s\n",
> expected_err, bpf_vlog);
> goto fail_log;
> diff --git a/tools/testing/selftests/bpf/verifier/wide_store.c b/tools/testing/selftests/bpf/verifier/wide_store.c
> new file mode 100644
> index 000000000000..c6385f45b114
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/verifier/wide_store.c
> @@ -0,0 +1,40 @@
> +#define BPF_SOCK_ADDR(field, off, res, err) \
> +{ \
> + "wide store to bpf_sock_addr." #field "[" #off "]", \
> + .insns = { \
> + BPF_MOV64_IMM(BPF_REG_0, 1), \
> + BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0, \
> + offsetof(struct bpf_sock_addr, field[off])), \
> + BPF_EXIT_INSN(), \
> + }, \
> + .result = res, \
> + .prog_type = BPF_PROG_TYPE_CGROUP_SOCK_ADDR, \
> + .expected_attach_type = BPF_CGROUP_UDP6_SENDMSG, \
> + .errstr = err, \
> +}
> +
> +/* user_ip6[0] is u64 aligned */
> +BPF_SOCK_ADDR(user_ip6, 0, ACCEPT,
> + NULL),
> +BPF_SOCK_ADDR(user_ip6, 1, REJECT,
> + "invalid bpf_context access off=12 size=8"),
> +BPF_SOCK_ADDR(user_ip6, 2, ACCEPT,
> + NULL),
> +BPF_SOCK_ADDR(user_ip6, 3, REJECT,
> + "invalid bpf_context access off=20 size=8"),
> +BPF_SOCK_ADDR(user_ip6, 4, REJECT,
> + "invalid bpf_context access off=24 size=8"),
With offset 4, we have
#968/p wide store to bpf_sock_addr.user_ip6[4] OK
This test case can be removed. user code typically
won't write bpf_sock_addr.user_ip6[4], and compiler
typically will give a warning since it is out of
array bound. Any particular reason you want to
include this one?
> +
> +/* msg_src_ip6[0] is _not_ u64 aligned */
> +BPF_SOCK_ADDR(msg_src_ip6, 0, REJECT,
> + "invalid bpf_context access off=44 size=8"),
> +BPF_SOCK_ADDR(msg_src_ip6, 1, ACCEPT,
> + NULL),
> +BPF_SOCK_ADDR(msg_src_ip6, 2, REJECT,
> + "invalid bpf_context access off=52 size=8"),
> +BPF_SOCK_ADDR(msg_src_ip6, 3, REJECT,
> + "invalid bpf_context access off=56 size=8"),
> +BPF_SOCK_ADDR(msg_src_ip6, 4, REJECT,
> + "invalid bpf_context access off=60 size=8"),
The same as above, offset=4 case can be removed?
> +
> +#undef BPF_SOCK_ADDR
>
^ permalink raw reply
* [PATCH net-next v2 00/16] mlxsw: PTP timestamping support
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
This is the second patchset adding PTP support in mlxsw. Next patchset
will add PTP shapers which are required to maintain accuracy under rates
lower than 40Gb/s, while subsequent patchsets will add tracepoints and
selftests.
Petr says:
This patch set introduces support for retrieving and processing hardware
timestamps for PTP packets.
The way PTP timestamping works on Spectrum-1 is that there are two queues
associated with each front panel port. When a packet is timestamped, the
timestamp is put to one of the queues: timestamps for transmitted packets
to one and for received packets to the other. Activity on these queues is
signaled through the events PTP_ING_FIFO and PTP_EGR_FIFO.
Packets themselves arrive through two traps: PTP0 and PTP1. It is possible
to configure which PTP messages should be trapped under which PTP trap. On
Spectrum systems, mlxsw will use PTP0 for event messages (which need
timestamping), and PTP1 for general messages (which do not).
There are therefore four relevant traps: receive of PTP event resp. general
message, and receive of timestamp for a transmitted resp. received PTP
packet. The obvious point where to put the new logic is a custom listener
to the mentioned traps.
Besides handling ingress traffic (be in packets or timestamps), the driver
also needs to handle timestamping of transmitted packets. One option would
be to invoke the relevant logic from mlxsw_core_ptp_transmitted(). However
on Spectrum-2, the timestamps are actually delivered through the completion
queue, and for that reason this patchset opts to invoke the logic from the
PCI code, via core and the driver, to a chip-specific operation. That way
the invocation will be done in a place where a Spectrum-2 implementation
will have an opportunity to extract the timestamp.
As indicated above, the PTP FIFO signaling happens independently from
packet delivery. A packet corresponding to any given timestamp could be
delivered sooner or later than the timestamp itself. Additionally, the
queues are only four elements deep, and it is therefore possible that the
timestamp for a delivered packet never arrives at all. Similarly a PTP
packet might be dropped due to CPU traffic pressure, and never be delivered
even if the corresponding timestamp was.
The driver thus needs to hold a cache of as-yet-unmatched SKBs and
timestamps. The first piece to arrive (be it timestamp or SKB) is put to
this cache. When the other piece arrives, the timestamp is attached to the
SKB and that is passed on. A delayed work is run at regular intervals to
prune the old unmatched entries.
As mentioned above, the mechanism for timestamp delivery changes on
Spectrum-2, where timestamps are part of completion queue elements, and all
packets are timestamped. All this bookkeeping is therefore unnecessary on
Spectrum-2. For this reason, this patchset spends some time introducing
Spectrum-1 specific artifacts such as a possibility to register a given
trap only on Spectrum-1.
Patches #1-#4 describe new registers.
Patches #5 and #6 introduce the possibility to register certain traps
only on some systems. The list of Spectrum-1 specific traps is left empty
at this point.
Patch #7 hooks into packet receive path by registering PTP traps
and appropriate handlers (that however do nothing of substance yet).
Patch #8 adds a helper to allow storing custom data to SKB->cb.
Patch #9 adds a call into the PCI completion queue handler that invokes,
via core and spectrum code, a PTP transmit handler. (Which also does not do
anything interesting yet.)
Patch #10 introduces code to invoke PTP initialization and adds data types
for the cache of unmatched entries.
Patches #11 and #12 implement the timestamping itself. In #11, the PHC
spin_locks are converted to _bh variants, because unlike normal PHC path,
which runs in process context, timestamp processing runs as soft interrupt.
Then #12 introduces the code for saving and retrieval of unmatched entries,
invokes PTP classifier to identify packets of interest, registers timestamp
FIFO events, and handles decoding and attaching timestamps to packets.
Patch #13 introduces a garbage collector for left-behind entries that have
not been matched for about a second.
In patch #14, PTP message types are configured to arrive as PTP0
(events) or PTP1 (everything else) as appropriate. At this point, the PTP
packets start arriving through the traps, but because PTP is disabled and
there is no way to enable it yet, they are always just passed to the usual
receive path right away.
Finally patches #15 and #16 add the plumbing to actually make it possible
to enable this code through SIOCSHWTSTAMP ioctl, and to advertise the
hardware timestamping capabilities through ethtool.
v2:
- Patch #12:
- In mlxsw_sp1_ptp_fifo_event_func(), post-increment when iterating over PTP
FIFO records.
- Patch #14:
- Change namespace of message type enumerators from MLXSW_ to MLXSW_SP_.
Petr Machata (16):
mlxsw: reg: Add Monitoring Time Precision Packet Port Configuration
Register
mlxsw: reg: Add Monitoring Precision Time Protocol Trap Register
mlxsw: reg: Add Time Precision Packet Timestamping Reading
mlxsw: reg: Add Monitoring Global Configuration Register
mlxsw: spectrum: Extract a helper for trap registration
mlxsw: spectrum: Add support for traps specific to Spectrum-1
mlxsw: spectrum: PTP: Hook into packet receive path
mlxsw: core: Add support for using SKB control buffer
mlxsw: pci: PTP: Hook into packet transmit path
mlxsw: spectrum: PTP: Add PTP initialization / finalization
mlxsw: spectrum: PTP: Disable BH when working with PHC
mlxsw: spectrum: PTP: Support timestamping on Spectrum-1
mlxsw: spectrum: PTP: Garbage-collect unmatched entries
mlxsw: spectrum: PTP: Configure PTP traps and FIFO events
mlxsw: spectrum: PTP: Support SIOCGHWTSTAMP, SIOCSHWTSTAMP ioctls
mlxsw: spectrum: PTP: Support ethtool get_ts_info
drivers/net/ethernet/mellanox/mlxsw/Kconfig | 1 +
drivers/net/ethernet/mellanox/mlxsw/core.c | 9 +
drivers/net/ethernet/mellanox/mlxsw/core.h | 19 +
drivers/net/ethernet/mellanox/mlxsw/pci.c | 17 +-
drivers/net/ethernet/mellanox/mlxsw/reg.h | 216 ++++++
.../net/ethernet/mellanox/mlxsw/spectrum.c | 290 ++++++-
.../net/ethernet/mellanox/mlxsw/spectrum.h | 11 +
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 711 +++++++++++++++++-
.../ethernet/mellanox/mlxsw/spectrum_ptp.h | 136 +++-
.../net/ethernet/mellanox/mlxsw/switchx2.c | 2 +
drivers/net/ethernet/mellanox/mlxsw/trap.h | 6 +
11 files changed, 1378 insertions(+), 40 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH net-next v2 01/16] mlxsw: reg: Add Monitoring Time Precision Packet Port Configuration Register
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
This register serves for configuration of which PTP messages should be
timestamped. This is a global configuration, despite the register name.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 38 +++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index e5f6bfd8a35a..971e336aa9ac 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -9148,6 +9148,43 @@ static inline void mlxsw_reg_mprs_pack(char *payload, u16 parsing_depth,
mlxsw_reg_mprs_vxlan_udp_dport_set(payload, vxlan_udp_dport);
}
+/* MTPPPC - Time Precision Packet Port Configuration
+ * -------------------------------------------------
+ * This register serves for configuration of which PTP messages should be
+ * timestamped. This is a global configuration, despite the register name.
+ *
+ * Reserved when Spectrum-2.
+ */
+#define MLXSW_REG_MTPPPC_ID 0x9090
+#define MLXSW_REG_MTPPPC_LEN 0x28
+
+MLXSW_REG_DEFINE(mtpppc, MLXSW_REG_MTPPPC_ID, MLXSW_REG_MTPPPC_LEN);
+
+/* reg_mtpppc_ing_timestamp_message_type
+ * Bitwise vector of PTP message types to timestamp at ingress.
+ * MessageType field as defined by IEEE 1588
+ * Each bit corresponds to a value (e.g. Bit0: Sync, Bit1: Delay_Req)
+ * Default all 0
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mtpppc, ing_timestamp_message_type, 0x08, 0, 16);
+
+/* reg_mtpppc_egr_timestamp_message_type
+ * Bitwise vector of PTP message types to timestamp at egress.
+ * MessageType field as defined by IEEE 1588
+ * Each bit corresponds to a value (e.g. Bit0: Sync, Bit1: Delay_Req)
+ * Default all 0
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mtpppc, egr_timestamp_message_type, 0x0C, 0, 16);
+
+static inline void mlxsw_reg_mtpppc_pack(char *payload, u16 ing, u16 egr)
+{
+ MLXSW_REG_ZERO(mtpppc, payload);
+ mlxsw_reg_mtpppc_ing_timestamp_message_type_set(payload, ing);
+ mlxsw_reg_mtpppc_egr_timestamp_message_type_set(payload, egr);
+}
+
/* MGPIR - Management General Peripheral Information Register
* ----------------------------------------------------------
* MGPIR register allows software to query the hardware and
@@ -10216,6 +10253,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
MLXSW_REG(mcda),
MLXSW_REG(mgpc),
MLXSW_REG(mprs),
+ MLXSW_REG(mtpppc),
MLXSW_REG(mgpir),
MLXSW_REG(tngcr),
MLXSW_REG(tnumt),
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 02/16] mlxsw: reg: Add Monitoring Precision Time Protocol Trap Register
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
This register is used for configuring under which trap to deliver PTP
packets depending on type of the packet.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 39 +++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 971e336aa9ac..5c5f63289468 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -9185,6 +9185,44 @@ static inline void mlxsw_reg_mtpppc_pack(char *payload, u16 ing, u16 egr)
mlxsw_reg_mtpppc_egr_timestamp_message_type_set(payload, egr);
}
+/* MTPTPT - Monitoring Precision Time Protocol Trap Register
+ * ---------------------------------------------------------
+ * This register is used for configuring under which trap to deliver PTP
+ * packets depending on type of the packet.
+ */
+#define MLXSW_REG_MTPTPT_ID 0x9092
+#define MLXSW_REG_MTPTPT_LEN 0x08
+
+MLXSW_REG_DEFINE(mtptpt, MLXSW_REG_MTPTPT_ID, MLXSW_REG_MTPTPT_LEN);
+
+enum mlxsw_reg_mtptpt_trap_id {
+ MLXSW_REG_MTPTPT_TRAP_ID_PTP0,
+ MLXSW_REG_MTPTPT_TRAP_ID_PTP1,
+};
+
+/* reg_mtptpt_trap_id
+ * Trap id.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mtptpt, trap_id, 0x00, 0, 4);
+
+/* reg_mtptpt_message_type
+ * Bitwise vector of PTP message types to trap. This is a necessary but
+ * non-sufficient condition since need to enable also per port. See MTPPPC.
+ * Message types are defined by IEEE 1588 Each bit corresponds to a value (e.g.
+ * Bit0: Sync, Bit1: Delay_Req)
+ */
+MLXSW_ITEM32(reg, mtptpt, message_type, 0x04, 0, 16);
+
+static inline void mlxsw_reg_mtptptp_pack(char *payload,
+ enum mlxsw_reg_mtptpt_trap_id trap_id,
+ u16 message_type)
+{
+ MLXSW_REG_ZERO(mtptpt, payload);
+ mlxsw_reg_mtptpt_trap_id_set(payload, trap_id);
+ mlxsw_reg_mtptpt_message_type_set(payload, message_type);
+}
+
/* MGPIR - Management General Peripheral Information Register
* ----------------------------------------------------------
* MGPIR register allows software to query the hardware and
@@ -10254,6 +10292,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
MLXSW_REG(mgpc),
MLXSW_REG(mprs),
MLXSW_REG(mtpppc),
+ MLXSW_REG(mtptpt),
MLXSW_REG(mgpir),
MLXSW_REG(tngcr),
MLXSW_REG(tnumt),
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 03/16] mlxsw: reg: Add Time Precision Packet Timestamping Reading
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
The MTPPTR is used for reading the per port PTP timestamp FIFO.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 110 ++++++++++++++++++++++
1 file changed, 110 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 5c5f63289468..197599890bdf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -9185,6 +9185,115 @@ static inline void mlxsw_reg_mtpppc_pack(char *payload, u16 ing, u16 egr)
mlxsw_reg_mtpppc_egr_timestamp_message_type_set(payload, egr);
}
+/* MTPPTR - Time Precision Packet Timestamping Reading
+ * ---------------------------------------------------
+ * The MTPPTR is used for reading the per port PTP timestamp FIFO.
+ * There is a trap for packets which are latched to the timestamp FIFO, thus the
+ * SW knows which FIFO to read. Note that packets enter the FIFO before been
+ * trapped. The sequence number is used to synchronize the timestamp FIFO
+ * entries and the trapped packets.
+ * Reserved when Spectrum-2.
+ */
+
+#define MLXSW_REG_MTPPTR_ID 0x9091
+#define MLXSW_REG_MTPPTR_BASE_LEN 0x10 /* base length, without records */
+#define MLXSW_REG_MTPPTR_REC_LEN 0x10 /* record length */
+#define MLXSW_REG_MTPPTR_REC_MAX_COUNT 4
+#define MLXSW_REG_MTPPTR_LEN (MLXSW_REG_MTPPTR_BASE_LEN + \
+ MLXSW_REG_MTPPTR_REC_LEN * MLXSW_REG_MTPPTR_REC_MAX_COUNT)
+
+MLXSW_REG_DEFINE(mtpptr, MLXSW_REG_MTPPTR_ID, MLXSW_REG_MTPPTR_LEN);
+
+/* reg_mtpptr_local_port
+ * Not supported for CPU port.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mtpptr, local_port, 0x00, 16, 8);
+
+enum mlxsw_reg_mtpptr_dir {
+ MLXSW_REG_MTPPTR_DIR_INGRESS,
+ MLXSW_REG_MTPPTR_DIR_EGRESS,
+};
+
+/* reg_mtpptr_dir
+ * Direction.
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mtpptr, dir, 0x00, 0, 1);
+
+/* reg_mtpptr_clr
+ * Clear the records.
+ * Access: OP
+ */
+MLXSW_ITEM32(reg, mtpptr, clr, 0x04, 31, 1);
+
+/* reg_mtpptr_num_rec
+ * Number of valid records in the response
+ * Range 0.. cap_ptp_timestamp_fifo
+ * Access: RO
+ */
+MLXSW_ITEM32(reg, mtpptr, num_rec, 0x08, 0, 4);
+
+/* reg_mtpptr_rec_message_type
+ * MessageType field as defined by IEEE 1588 Each bit corresponds to a value
+ * (e.g. Bit0: Sync, Bit1: Delay_Req)
+ * Access: RO
+ */
+MLXSW_ITEM32_INDEXED(reg, mtpptr, rec_message_type,
+ MLXSW_REG_MTPPTR_BASE_LEN, 8, 4,
+ MLXSW_REG_MTPPTR_REC_LEN, 0, false);
+
+/* reg_mtpptr_rec_domain_number
+ * DomainNumber field as defined by IEEE 1588
+ * Access: RO
+ */
+MLXSW_ITEM32_INDEXED(reg, mtpptr, rec_domain_number,
+ MLXSW_REG_MTPPTR_BASE_LEN, 0, 8,
+ MLXSW_REG_MTPPTR_REC_LEN, 0, false);
+
+/* reg_mtpptr_rec_sequence_id
+ * SequenceId field as defined by IEEE 1588
+ * Access: RO
+ */
+MLXSW_ITEM32_INDEXED(reg, mtpptr, rec_sequence_id,
+ MLXSW_REG_MTPPTR_BASE_LEN, 0, 16,
+ MLXSW_REG_MTPPTR_REC_LEN, 0x4, false);
+
+/* reg_mtpptr_rec_timestamp_high
+ * Timestamp of when the PTP packet has passed through the port Units of PLL
+ * clock time.
+ * For Spectrum-1 the PLL clock is 156.25Mhz and PLL clock time is 6.4nSec.
+ * Access: RO
+ */
+MLXSW_ITEM32_INDEXED(reg, mtpptr, rec_timestamp_high,
+ MLXSW_REG_MTPPTR_BASE_LEN, 0, 32,
+ MLXSW_REG_MTPPTR_REC_LEN, 0x8, false);
+
+/* reg_mtpptr_rec_timestamp_low
+ * See rec_timestamp_high.
+ * Access: RO
+ */
+MLXSW_ITEM32_INDEXED(reg, mtpptr, rec_timestamp_low,
+ MLXSW_REG_MTPPTR_BASE_LEN, 0, 32,
+ MLXSW_REG_MTPPTR_REC_LEN, 0xC, false);
+
+static inline void mlxsw_reg_mtpptr_unpack(const char *payload,
+ unsigned int rec,
+ u8 *p_message_type,
+ u8 *p_domain_number,
+ u16 *p_sequence_id,
+ u64 *p_timestamp)
+{
+ u32 timestamp_high, timestamp_low;
+
+ *p_message_type = mlxsw_reg_mtpptr_rec_message_type_get(payload, rec);
+ *p_domain_number = mlxsw_reg_mtpptr_rec_domain_number_get(payload, rec);
+ *p_sequence_id = mlxsw_reg_mtpptr_rec_sequence_id_get(payload, rec);
+ timestamp_high = mlxsw_reg_mtpptr_rec_timestamp_high_get(payload, rec);
+ timestamp_low = mlxsw_reg_mtpptr_rec_timestamp_low_get(payload, rec);
+ *p_timestamp = (u64)timestamp_high << 32 | timestamp_low;
+}
+
/* MTPTPT - Monitoring Precision Time Protocol Trap Register
* ---------------------------------------------------------
* This register is used for configuring under which trap to deliver PTP
@@ -10292,6 +10401,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
MLXSW_REG(mgpc),
MLXSW_REG(mprs),
MLXSW_REG(mtpppc),
+ MLXSW_REG(mtpptr),
MLXSW_REG(mtptpt),
MLXSW_REG(mgpir),
MLXSW_REG(tngcr),
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 04/16] mlxsw: reg: Add Monitoring Global Configuration Register
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
This register serves to configure global parameters of certain
monitoring operations. The following patches will use it to configure
that when PTP timestamps are delivered through the PTP FIFO traps, the
FIFO in question is cleared as well.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 27 +++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 197599890bdf..8de9333e6eb1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -9148,6 +9148,32 @@ static inline void mlxsw_reg_mprs_pack(char *payload, u16 parsing_depth,
mlxsw_reg_mprs_vxlan_udp_dport_set(payload, vxlan_udp_dport);
}
+/* MOGCR - Monitoring Global Configuration Register
+ * ------------------------------------------------
+ */
+#define MLXSW_REG_MOGCR_ID 0x9086
+#define MLXSW_REG_MOGCR_LEN 0x20
+
+MLXSW_REG_DEFINE(mogcr, MLXSW_REG_MOGCR_ID, MLXSW_REG_MOGCR_LEN);
+
+/* reg_mogcr_ptp_iftc
+ * PTP Ingress FIFO Trap Clear
+ * The PTP_ING_FIFO trap provides MTPPTR with clr according
+ * to this value. Default 0.
+ * Reserved when IB switches and when SwitchX/-2, Spectrum-2
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mogcr, ptp_iftc, 0x00, 1, 1);
+
+/* reg_mogcr_ptp_eftc
+ * PTP Egress FIFO Trap Clear
+ * The PTP_EGR_FIFO trap provides MTPPTR with clr according
+ * to this value. Default 0.
+ * Reserved when IB switches and when SwitchX/-2, Spectrum-2
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mogcr, ptp_eftc, 0x00, 0, 1);
+
/* MTPPPC - Time Precision Packet Port Configuration
* -------------------------------------------------
* This register serves for configuration of which PTP messages should be
@@ -10400,6 +10426,7 @@ static const struct mlxsw_reg_info *mlxsw_reg_infos[] = {
MLXSW_REG(mcda),
MLXSW_REG(mgpc),
MLXSW_REG(mprs),
+ MLXSW_REG(mogcr),
MLXSW_REG(mtpppc),
MLXSW_REG(mtpptr),
MLXSW_REG(mtptpt),
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 05/16] mlxsw: spectrum: Extract a helper for trap registration
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
On Spectrum-1, timestamps for PTP packets are delivered through queues
of ingress and egress timestamps. There are two event traps
corresponding to activity on each of those queues. This mechanism is
absent on Spectrum-2, and therefore the traps should only be registered
on Spectrum-1.
Extract out of mlxsw_sp_traps_init() a generic helper,
mlxsw_sp_traps_register(), and likewise with _unregister(). The new helpers
will later be called with Spectrum-1-specific traps.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.c | 48 +++++++++++++------
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 3e8593824b33..0119efe0ea7a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4251,22 +4251,16 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
return 0;
}
-static int mlxsw_sp_traps_init(struct mlxsw_sp *mlxsw_sp)
+static int mlxsw_sp_traps_register(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_listener listeners[],
+ size_t listeners_count)
{
int i;
int err;
- err = mlxsw_sp_cpu_policers_set(mlxsw_sp->core);
- if (err)
- return err;
-
- err = mlxsw_sp_trap_groups_set(mlxsw_sp->core);
- if (err)
- return err;
-
- for (i = 0; i < ARRAY_SIZE(mlxsw_sp_listener); i++) {
+ for (i = 0; i < listeners_count; i++) {
err = mlxsw_core_trap_register(mlxsw_sp->core,
- &mlxsw_sp_listener[i],
+ &listeners[i],
mlxsw_sp);
if (err)
goto err_listener_register;
@@ -4277,23 +4271,47 @@ static int mlxsw_sp_traps_init(struct mlxsw_sp *mlxsw_sp)
err_listener_register:
for (i--; i >= 0; i--) {
mlxsw_core_trap_unregister(mlxsw_sp->core,
- &mlxsw_sp_listener[i],
+ &listeners[i],
mlxsw_sp);
}
return err;
}
-static void mlxsw_sp_traps_fini(struct mlxsw_sp *mlxsw_sp)
+static void mlxsw_sp_traps_unregister(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_listener listeners[],
+ size_t listeners_count)
{
int i;
- for (i = 0; i < ARRAY_SIZE(mlxsw_sp_listener); i++) {
+ for (i = 0; i < listeners_count; i++) {
mlxsw_core_trap_unregister(mlxsw_sp->core,
- &mlxsw_sp_listener[i],
+ &listeners[i],
mlxsw_sp);
}
}
+static int mlxsw_sp_traps_init(struct mlxsw_sp *mlxsw_sp)
+{
+ int err;
+
+ err = mlxsw_sp_cpu_policers_set(mlxsw_sp->core);
+ if (err)
+ return err;
+
+ err = mlxsw_sp_trap_groups_set(mlxsw_sp->core);
+ if (err)
+ return err;
+
+ return mlxsw_sp_traps_register(mlxsw_sp, mlxsw_sp_listener,
+ ARRAY_SIZE(mlxsw_sp_listener));
+}
+
+static void mlxsw_sp_traps_fini(struct mlxsw_sp *mlxsw_sp)
+{
+ mlxsw_sp_traps_unregister(mlxsw_sp, mlxsw_sp_listener,
+ ARRAY_SIZE(mlxsw_sp_listener));
+}
+
#define MLXSW_SP_LAG_SEED_INIT 0xcafecafe
static int mlxsw_sp_lag_init(struct mlxsw_sp *mlxsw_sp)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 06/16] mlxsw: spectrum: Add support for traps specific to Spectrum-1
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
On Spectrum-1, timestamps for PTP packets are delivered through queues
of ingress and egress timestamps. There are two event traps
corresponding to activity on each of those queues. This mechanism is
absent on Spectrum-2, and therefore the traps should only be registered
on Spectrum-1.
Carry a chip-specific listener array in mlxsw_sp->listeners and
listeners_count. Register listeners from that array in
mlxsw_sp_traps_init(). Add a new listener array for Spectrum-1 traps and
configure the newly-added mlxsw_sp->listeners with this array.
The listener array is empty for now, the events will be added in a later
patch.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.c | 25 +++++++++++++++++--
.../net/ethernet/mellanox/mlxsw/spectrum.h | 2 ++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 0119efe0ea7a..91486193454a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4114,6 +4114,9 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
MLXSW_SP_RXL_NO_MARK(NVE_DECAP_ARP, TRAP_TO_CPU, ARP, false),
};
+static const struct mlxsw_listener mlxsw_sp1_listener[] = {
+};
+
static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
{
char qpcr_pl[MLXSW_REG_QPCR_LEN];
@@ -4302,12 +4305,28 @@ static int mlxsw_sp_traps_init(struct mlxsw_sp *mlxsw_sp)
if (err)
return err;
- return mlxsw_sp_traps_register(mlxsw_sp, mlxsw_sp_listener,
- ARRAY_SIZE(mlxsw_sp_listener));
+ err = mlxsw_sp_traps_register(mlxsw_sp, mlxsw_sp_listener,
+ ARRAY_SIZE(mlxsw_sp_listener));
+ if (err)
+ return err;
+
+ err = mlxsw_sp_traps_register(mlxsw_sp, mlxsw_sp->listeners,
+ mlxsw_sp->listeners_count);
+ if (err)
+ goto err_extra_traps_init;
+
+ return 0;
+
+err_extra_traps_init:
+ mlxsw_sp_traps_unregister(mlxsw_sp, mlxsw_sp_listener,
+ ARRAY_SIZE(mlxsw_sp_listener));
+ return err;
}
static void mlxsw_sp_traps_fini(struct mlxsw_sp *mlxsw_sp)
{
+ mlxsw_sp_traps_unregister(mlxsw_sp, mlxsw_sp->listeners,
+ mlxsw_sp->listeners_count);
mlxsw_sp_traps_unregister(mlxsw_sp, mlxsw_sp_listener,
ARRAY_SIZE(mlxsw_sp_listener));
}
@@ -4566,6 +4585,8 @@ static int mlxsw_sp1_init(struct mlxsw_core *mlxsw_core,
mlxsw_sp->sb_vals = &mlxsw_sp1_sb_vals;
mlxsw_sp->port_type_speed_ops = &mlxsw_sp1_port_type_speed_ops;
mlxsw_sp->ptp_ops = &mlxsw_sp1_ptp_ops;
+ mlxsw_sp->listeners = mlxsw_sp1_listener;
+ mlxsw_sp->listeners_count = ARRAY_SIZE(mlxsw_sp1_listener);
return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 84f4276193b3..9136a86dc55f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -175,6 +175,8 @@ struct mlxsw_sp {
const struct mlxsw_sp_sb_vals *sb_vals;
const struct mlxsw_sp_port_type_speed_ops *port_type_speed_ops;
const struct mlxsw_sp_ptp_ops *ptp_ops;
+ const struct mlxsw_listener *listeners;
+ size_t listeners_count;
};
static inline struct mlxsw_sp_upper *
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 07/16] mlxsw: spectrum: PTP: Hook into packet receive path
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
When configured, the Spectrum hardware can recognize PTP packets and
trap them to the CPU using dedicated traps, PTP0 and PTP1.
One reason to get PTP packets under dedicated traps is to have a
separate policer suitable for the amount of PTP traffic expected when
switch is operated as a boundary clock. For this, add two new trap
groups, MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP0 and _PTP1, and associate the
two PTP traps with these two groups.
In the driver, specifically for Spectrum-1, event PTP packets will need
to be paired up with their timestamps. Those arrive through a different
set of traps, added later in the patch set. To support this future use,
introduce a new PTP op, ptp_receive.
It is possible to configure which PTP messages should be trapped under
which PTP trap. On Spectrum systems, we will use PTP0 for event
packets (which need timestamping), and PTP1 for control packets (which
do not). Thus configure PTP0 trap with a custom callback that defers to
the ptp_receive op.
Additionally, L2 PTP packets are actually trapped through the LLDP trap,
not through any of the PTP traps. So treat the LLDP trap the same way as
the PTP0 trap. Unlike PTP traps, which are currently still disabled,
LLDP trap is active. Correspondingly, have all the implementations of
the ptp_receive op return true, which the handler treats as a signal to
forward the packet immediately.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 2 +
.../net/ethernet/mellanox/mlxsw/spectrum.c | 49 +++++++++++++++----
.../net/ethernet/mellanox/mlxsw/spectrum.h | 2 +
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 6 +++
.../ethernet/mellanox/mlxsw/spectrum_ptp.h | 15 ++++++
drivers/net/ethernet/mellanox/mlxsw/trap.h | 2 +
6 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 8de9333e6eb1..76ff5b217c04 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5292,6 +5292,8 @@ enum mlxsw_reg_htgt_trap_group {
MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_MLD,
MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_ND,
MLXSW_REG_HTGT_TRAP_GROUP_SP_LBERROR,
+ MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP0,
+ MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP1,
};
/* reg_htgt_trap_group
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 91486193454a..84f4077b4b37 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -147,6 +147,18 @@ struct mlxsw_sp_mlxfw_dev {
struct mlxsw_sp *mlxsw_sp;
};
+struct mlxsw_sp_ptp_ops {
+ struct mlxsw_sp_ptp_clock *
+ (*clock_init)(struct mlxsw_sp *mlxsw_sp, struct device *dev);
+ void (*clock_fini)(struct mlxsw_sp_ptp_clock *clock);
+
+ /* Notify a driver that a packet that might be PTP was received. Driver
+ * is responsible for freeing the passed-in SKB.
+ */
+ void (*receive)(struct mlxsw_sp *mlxsw_sp, struct sk_buff *skb,
+ u8 local_port);
+};
+
static int mlxsw_sp_component_query(struct mlxfw_dev *mlxfw_dev,
u16 component_index, u32 *p_max_size,
u8 *p_align_bits, u16 *p_max_write_size)
@@ -3947,8 +3959,8 @@ static void mlxsw_sp_pude_event_func(const struct mlxsw_reg_info *reg,
}
}
-static void mlxsw_sp_rx_listener_no_mark_func(struct sk_buff *skb,
- u8 local_port, void *priv)
+void mlxsw_sp_rx_listener_no_mark_func(struct sk_buff *skb,
+ u8 local_port, void *priv)
{
struct mlxsw_sp *mlxsw_sp = priv;
struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp->ports[local_port];
@@ -4022,6 +4034,14 @@ static void mlxsw_sp_rx_listener_sample_func(struct sk_buff *skb, u8 local_port,
consume_skb(skb);
}
+static void mlxsw_sp_rx_listener_ptp(struct sk_buff *skb, u8 local_port,
+ void *priv)
+{
+ struct mlxsw_sp *mlxsw_sp = priv;
+
+ mlxsw_sp->ptp_ops->receive(mlxsw_sp, skb, local_port);
+}
+
#define MLXSW_SP_RXL_NO_MARK(_trap_id, _action, _trap_group, _is_ctrl) \
MLXSW_RXL(mlxsw_sp_rx_listener_no_mark_func, _trap_id, _action, \
_is_ctrl, SP_##_trap_group, DISCARD)
@@ -4043,7 +4063,8 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
/* L2 traps */
MLXSW_SP_RXL_NO_MARK(STP, TRAP_TO_CPU, STP, true),
MLXSW_SP_RXL_NO_MARK(LACP, TRAP_TO_CPU, LACP, true),
- MLXSW_SP_RXL_NO_MARK(LLDP, TRAP_TO_CPU, LLDP, true),
+ MLXSW_RXL(mlxsw_sp_rx_listener_ptp, LLDP, TRAP_TO_CPU,
+ false, SP_LLDP, DISCARD),
MLXSW_SP_RXL_MARK(DHCP, MIRROR_TO_CPU, DHCP, false),
MLXSW_SP_RXL_MARK(IGMP_QUERY, MIRROR_TO_CPU, IGMP, false),
MLXSW_SP_RXL_NO_MARK(IGMP_V1_REPORT, TRAP_TO_CPU, IGMP, false),
@@ -4112,6 +4133,10 @@ static const struct mlxsw_listener mlxsw_sp_listener[] = {
/* NVE traps */
MLXSW_SP_RXL_MARK(NVE_ENCAP_ARP, TRAP_TO_CPU, ARP, false),
MLXSW_SP_RXL_NO_MARK(NVE_DECAP_ARP, TRAP_TO_CPU, ARP, false),
+ /* PTP traps */
+ MLXSW_RXL(mlxsw_sp_rx_listener_ptp, PTP0, TRAP_TO_CPU,
+ false, SP_PTP0, DISCARD),
+ MLXSW_SP_RXL_NO_MARK(PTP1, TRAP_TO_CPU, PTP1, false),
};
static const struct mlxsw_listener mlxsw_sp1_listener[] = {
@@ -4166,6 +4191,14 @@ static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
rate = 1024;
burst_size = 7;
break;
+ case MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP0:
+ rate = 24 * 1024;
+ burst_size = 12;
+ break;
+ case MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP1:
+ rate = 19 * 1024;
+ burst_size = 12;
+ break;
default:
continue;
}
@@ -4204,6 +4237,7 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
case MLXSW_REG_HTGT_TRAP_GROUP_SP_LLDP:
case MLXSW_REG_HTGT_TRAP_GROUP_SP_OSPF:
case MLXSW_REG_HTGT_TRAP_GROUP_SP_PIM:
+ case MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP0:
priority = 5;
tc = 5;
break;
@@ -4221,6 +4255,7 @@ static int mlxsw_sp_trap_groups_set(struct mlxsw_core *mlxsw_core)
case MLXSW_REG_HTGT_TRAP_GROUP_SP_ARP:
case MLXSW_REG_HTGT_TRAP_GROUP_SP_IPV6_ND:
case MLXSW_REG_HTGT_TRAP_GROUP_SP_RPF:
+ case MLXSW_REG_HTGT_TRAP_GROUP_SP_PTP1:
priority = 2;
tc = 2;
break;
@@ -4383,20 +4418,16 @@ static int mlxsw_sp_basic_trap_groups_set(struct mlxsw_core *mlxsw_core)
return mlxsw_reg_write(mlxsw_core, MLXSW_REG(htgt), htgt_pl);
}
-struct mlxsw_sp_ptp_ops {
- struct mlxsw_sp_ptp_clock *
- (*clock_init)(struct mlxsw_sp *mlxsw_sp, struct device *dev);
- void (*clock_fini)(struct mlxsw_sp_ptp_clock *clock);
-};
-
static const struct mlxsw_sp_ptp_ops mlxsw_sp1_ptp_ops = {
.clock_init = mlxsw_sp1_ptp_clock_init,
.clock_fini = mlxsw_sp1_ptp_clock_fini,
+ .receive = mlxsw_sp1_ptp_receive,
};
static const struct mlxsw_sp_ptp_ops mlxsw_sp2_ptp_ops = {
.clock_init = mlxsw_sp2_ptp_clock_init,
.clock_fini = mlxsw_sp2_ptp_clock_fini,
+ .receive = mlxsw_sp2_ptp_receive,
};
static int mlxsw_sp_netdevice_event(struct notifier_block *unused,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 9136a86dc55f..139fb1c53f96 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -440,6 +440,8 @@ struct mlxsw_sp_fid *mlxsw_sp_bridge_fid_get(struct mlxsw_sp *mlxsw_sp,
extern struct notifier_block mlxsw_sp_switchdev_notifier;
/* spectrum.c */
+void mlxsw_sp_rx_listener_no_mark_func(struct sk_buff *skb,
+ u8 local_port, void *priv);
int mlxsw_sp_port_ets_set(struct mlxsw_sp_port *mlxsw_sp_port,
enum mlxsw_reg_qeec_hr hr, u8 index, u8 next_index,
bool dwrr, u8 dwrr_weight);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index bb6c0cb25771..8eca1ac03e7a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -264,3 +264,9 @@ void mlxsw_sp1_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock)
cancel_delayed_work_sync(&clock->overflow_work);
kfree(clock);
}
+
+void mlxsw_sp1_ptp_receive(struct mlxsw_sp *mlxsw_sp, struct sk_buff *skb,
+ u8 local_port)
+{
+ mlxsw_sp_rx_listener_no_mark_func(skb, local_port, mlxsw_sp);
+}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
index 76fa00a4be75..06bb303b5407 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
@@ -17,6 +17,9 @@ mlxsw_sp1_ptp_clock_init(struct mlxsw_sp *mlxsw_sp, struct device *dev);
void mlxsw_sp1_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock);
+void mlxsw_sp1_ptp_receive(struct mlxsw_sp *mlxsw_sp, struct sk_buff *skb,
+ u8 local_port);
+
#else
static inline struct mlxsw_sp_ptp_clock *
@@ -29,6 +32,12 @@ static inline void mlxsw_sp1_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock)
{
}
+static inline void mlxsw_sp1_ptp_receive(struct mlxsw_sp *mlxsw_sp,
+ struct sk_buff *skb, u8 local_port)
+{
+ mlxsw_sp_rx_listener_no_mark_func(skb, local_port, mlxsw_sp);
+}
+
#endif
static inline struct mlxsw_sp_ptp_clock *
@@ -41,4 +50,10 @@ static inline void mlxsw_sp2_ptp_clock_fini(struct mlxsw_sp_ptp_clock *clock)
{
}
+static inline void mlxsw_sp2_ptp_receive(struct mlxsw_sp *mlxsw_sp,
+ struct sk_buff *skb, u8 local_port)
+{
+ mlxsw_sp_rx_listener_no_mark_func(skb, local_port, mlxsw_sp);
+}
+
#endif
diff --git a/drivers/net/ethernet/mellanox/mlxsw/trap.h b/drivers/net/ethernet/mellanox/mlxsw/trap.h
index 451216dd7f6b..f05b7ff4b9df 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/trap.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/trap.h
@@ -17,6 +17,8 @@ enum {
MLXSW_TRAP_ID_MVRP = 0x15,
MLXSW_TRAP_ID_RPVST = 0x16,
MLXSW_TRAP_ID_DHCP = 0x19,
+ MLXSW_TRAP_ID_PTP0 = 0x28,
+ MLXSW_TRAP_ID_PTP1 = 0x29,
MLXSW_TRAP_ID_IGMP_QUERY = 0x30,
MLXSW_TRAP_ID_IGMP_V1_REPORT = 0x31,
MLXSW_TRAP_ID_IGMP_V2_REPORT = 0x32,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v2 08/16] mlxsw: core: Add support for using SKB control buffer
From: Ido Schimmel @ 2019-06-30 6:04 UTC (permalink / raw)
To: netdev; +Cc: davem, richardcochran, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
The SKB control buffer is useful (and used) for bookkeeping of information
related to that SKB. Add helpers so that the mlxsw driver(s) can safely use
the buffer as well. The structure is currently empty, individual users will
add members to it as necessary.
Note that SKB allocation functions already clear the buffer, so the cleanup
is only necessary when ndo_start_xmit is called.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.h | 9 +++++++++
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 2 ++
drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 2 ++
3 files changed, 13 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 6dbb0ede502e..06babcc58c7a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -418,4 +418,13 @@ enum mlxsw_devlink_param_id {
MLXSW_DEVLINK_PARAM_ID_ACL_REGION_REHASH_INTERVAL,
};
+struct mlxsw_skb_cb {
+};
+
+static inline struct mlxsw_skb_cb *mlxsw_skb_cb(struct sk_buff *skb)
+{
+ BUILD_BUG_ON(sizeof(mlxsw_skb_cb) > sizeof(skb->cb));
+ return (struct mlxsw_skb_cb *) skb->cb;
+}
+
#endif
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 84f4077b4b37..a0376d4f94a8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -790,6 +790,8 @@ static netdev_tx_t mlxsw_sp_port_xmit(struct sk_buff *skb,
u64 len;
int err;
+ memset(skb->cb, 0, sizeof(struct mlxsw_skb_cb));
+
if (mlxsw_core_skb_transmit_busy(mlxsw_sp->core, &tx_info))
return NETDEV_TX_BUSY;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index fc4f19167262..bdab96f5bc70 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -299,6 +299,8 @@ static netdev_tx_t mlxsw_sx_port_xmit(struct sk_buff *skb,
u64 len;
int err;
+ memset(skb->cb, 0, sizeof(struct mlxsw_skb_cb));
+
if (mlxsw_core_skb_transmit_busy(mlxsw_sx->core, &tx_info))
return NETDEV_TX_BUSY;
--
2.20.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