* Re: [PATCH bpf-next 05/13] bpf: adding map batch processing support
From: Yonghong Song @ 2019-08-30 6:39 UTC (permalink / raw)
To: Brian Vazquez
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <CAMzD94RuPs8_BHNRDjk6NDkyi=hJ+pCKeLb3ihACLYaOWz8sAA@mail.gmail.com>
On 8/29/19 4:01 PM, Brian Vazquez wrote:
> Hi Yonghong!
>
> Thanks for sending this series of patches and starting the discussion.
>
> On Wed, Aug 28, 2019 at 11:45 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> Brian Vazquez has proposed BPF_MAP_DUMP command to look up more than one
>> map entries per syscall.
>> https://lore.kernel.org/bpf/CABCgpaU3xxX6CMMxD+1knApivtc2jLBHysDXw-0E9bQEL0qC3A@mail.gmail.com/T/#t
>>
>> During discussion, we found more use cases can be supported in a similar
>> map operation batching framework. For example, batched map lookup and delete,
>> which can be really helpful for bcc.
>> https://github.com/iovisor/bcc/blob/master/tools/tcptop.py#L233-L243
>> https://github.com/iovisor/bcc/blob/master/tools/slabratetop.py#L129-L138
>>
>> Also, in bcc, we have API to delete all entries in a map.
>> https://github.com/iovisor/bcc/blob/master/src/cc/api/BPFTable.h#L257-L264
>>
>> For map update, batched operations also useful as sometimes applications need
>> to populate initial maps with more than one entry. For example, the below
>> example is from kernel/samples/bpf/xdp_redirect_cpu_user.c:
>> https://github.com/torvalds/linux/blob/master/samples/bpf/xdp_redirect_cpu_user.c#L543-L550
>>
>> This patch addresses all the above use cases. To make uapi stable, it also
>> covers other potential use cases. Four bpf syscall subcommands are introduced:
>> BPF_MAP_LOOKUP_BATCH
>> BPF_MAP_LOOKUP_AND_DELETE_BATCH
>> BPF_MAP_UPDATE_BATCH
>> BPF_MAP_DELETE_BATCH
>>
>> The UAPI attribute structure looks like:
>> struct { /* struct used by BPF_MAP_*_BATCH commands */
>> __aligned_u64 start_key; /* input: storing start key,
>> * if NULL, starting from the beginning.
>> */
>> __aligned_u64 next_start_key; /* output: storing next batch start_key,
>> * if NULL, no next key.
>> */
>> __aligned_u64 keys; /* input/output: key buffer */
>> __aligned_u64 values; /* input/output: value buffer */
>> __u32 count; /* input: # of keys/values in
>> * or fits in keys[]/values[].
>> * output: how many successful
>> * lookup/lookup_and_delete
>> * /delete/update operations.
>> */
>> __u32 map_fd;
>> __u64 elem_flags; /* BPF_MAP_{UPDATE,LOOKUP}_ELEM flags */
>> __u64 flags; /* flags for batch operation */
>> } batch;
>>
>> The 'start_key' and 'next_start_key' are used to BPF_MAP_LOOKUP_BATCH,
>> BPF_MAP_LOOKUP_AND_DELETE_BATCH and BPF_MAP_DELETE_BATCH
>> to start the operation on 'start_key' and also set the
>> next batch start key in 'next_start_key'.
>>
>> If 'count' is greater than 0 and the return code is 0,
>> . the 'count' may be updated to be smaller if there is less
>> elements than 'count' for the operation. In such cases,
>> 'next_start_key' will be set to NULL.
>> . the 'count' remains the same. 'next_start_key' could be NULL
>> or could point to next start_key for batch processing.
>>
>> If 'count' is greater than 0 and the return code is an error
>> other than -EFAULT, the kernel will try to overwrite 'count'
>> to contain the number of successful element-level (lookup,
>> lookup_and_delete, update and delete) operations. The following
>> attributes can be checked:
>> . if 'count' value is modified, the new value will be
>> the number of successful element-level operations.
>> . if 'count' value is modified, the keys[]/values[] will
>> contain correct results for new 'count' number of
>> operations for lookup[_and_delete] and update.
>>
>> The implementation in this patch mimics what user space
>> did, e.g., for lookup_and_delete,
>> while(bpf_get_next_key(map, keyp, next_keyp) == 0) {
>> bpf_map_delete_elem(map, keyp);
>> bpf_map_lookup_elem(map, next_keyp, &value, flags);
>> keyp, next_keyp = next_keyp, keyp;
>> }
>> The similar loop is implemented in the kernel, and
>> each operation, bpf_get_next_key(), bpf_map_delete_elem()
>> and bpf_map_lookup_elem(), uses existing kernel functions
>> each of which has its own rcu_read_lock region, bpf_prog_active
>> protection, etc.
>> Therefore, it is totally possible that after bpf_get_next_key(),
>> the bpf_map_delete_elem() or bpf_map_lookup_elem() may fail
>> as the key may be deleted concurrently by kernel or
>> other user space processes/threads.
>> By default, the current implementation permits the loop
>> to continue, just like typical user space handling. But
>> a flag, BPF_F_ENFORCE_ENOENT, can be specified, so kernel
>> will return an error if bpf_map_delete_elem() or
>> bpf_map_lookup_elem() failed.
>>
>> The high-level algorithm for BPF_MAP_LOOKUP_BATCH and
>> BPF_MAP_LOOKUP_AND_DELETE_BATCH:
>> if (start_key == NULL and next_start_key == NULL) {
>> lookup up to 'count' keys in keys[] and fill
>> corresponding values[], and delete those
>> keys if BPF_MAP_LOOKUP_AND_DELETE_BATCH.
>> } else if (start_key == NULL && next_start_key != NULL) {
>> lookup up to 'count' keys from the beginning
>> of the map and fill keys[]/values[], delete
>> those keys if BPF_MAP_LOOKUP_AND_DELETE_BATCH.
>> Set 'next_start_key' for next batch operation.
>> } else if (start_key != NULL && next_start_key != NULL) {
>> lookup to 'count' keys from 'start_key', inclusive,
>> and fill keys[]/values[], delete those keys if
>> BPF_MAP_LOOKUP_AND_DELETE_BATCH.
>> Set 'next_start_key' for next batch operation.
>> }
>>
>> The high-level algorithm for BPF_MAP_UPDATE_BATCH:
>> if (count != 0) {
>> do 'count' number of updates on keys[]/values[].
>> }
>>
>> The high-level algorithm for BPF_MAP_DELETE_BATCH:
>> if (count == 0) {
>> if (start_key == NULL) {
>> delete all elements from map.
>> } else {
>> delete all elements from start_key to the end of map.
>> }
>> } else {
>> if (start_key == NULL and next_start_key == NULL) {
>> delete 'count' number of keys in keys[].
>> } else if (start_key == NULL and next_start_key != NULL) {
>> delete 'count' number of keys from the
>> beginning of the map and set 'next_start_key'
>> properly.
>> } else if (start_key != NULL and next_start_keeey != NULL) {
>> delete 'count' number of keys from 'start_key',
>> and set 'next_start_key' properly.
>> }
>> }
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> include/uapi/linux/bpf.h | 27 +++
>> kernel/bpf/syscall.c | 448 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 475 insertions(+)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 5d2fb183ee2d..576688f13e8c 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -107,6 +107,10 @@ enum bpf_cmd {
>> BPF_MAP_LOOKUP_AND_DELETE_ELEM,
>> BPF_MAP_FREEZE,
>> BPF_BTF_GET_NEXT_ID,
>> + BPF_MAP_LOOKUP_BATCH,
>> + BPF_MAP_LOOKUP_AND_DELETE_BATCH,
>> + BPF_MAP_UPDATE_BATCH,
>> + BPF_MAP_DELETE_BATCH,
>> };
>>
>> enum bpf_map_type {
>> @@ -347,6 +351,9 @@ enum bpf_attach_type {
>> /* flags for BPF_PROG_QUERY */
>> #define BPF_F_QUERY_EFFECTIVE (1U << 0)
>>
>> +/* flags for BPF_MAP_*_BATCH */
>> +#define BPF_F_ENFORCE_ENOENT (1U << 0)
>> +
>> enum bpf_stack_build_id_status {
>> /* user space need an empty entry to identify end of a trace */
>> BPF_STACK_BUILD_ID_EMPTY = 0,
>> @@ -396,6 +403,26 @@ union bpf_attr {
>> __u64 flags;
>> };
>>
>> + struct { /* struct used by BPF_MAP_*_BATCH commands */
>> + __aligned_u64 start_key; /* input: storing start key,
>> + * if NULL, starting from the beginning.
>> + */
>> + __aligned_u64 next_start_key; /* output: storing next batch start_key,
>> + * if NULL, no next key.
>> + */
>> + __aligned_u64 keys; /* input/output: key buffer */
>> + __aligned_u64 values; /* input/output: value buffer */
>> + __u32 count; /* input: # of keys/values in
>> + * or fits in keys[]/values[].
>> + * output: how many successful
>> + * lookup/lookup_and_delete
>> + * update/delete operations.
>> + */
>> + __u32 map_fd;
>> + __u64 elem_flags; /* BPF_MAP_*_ELEM flags */
>> + __u64 flags; /* flags for batch operation */
>> + } batch;
>> +
>> struct { /* anonymous struct used by BPF_PROG_LOAD command */
>> __u32 prog_type; /* one of enum bpf_prog_type */
>> __u32 insn_cnt;
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 06308f0206e5..8746b55405f9 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -33,6 +33,17 @@
>>
>> #define BPF_OBJ_FLAG_MASK (BPF_F_RDONLY | BPF_F_WRONLY)
>>
>> +#define BPF_MAP_BATCH_SWAP_KEYS(key1, key2, buf1, buf2) \
>> + do { \
>> + if (key1 == (buf1)) { \
>> + key1 = buf2; \
>> + key2 = buf1; \
>> + } else { \
>> + key1 = buf1; \
>> + key2 = buf2; \
>> + } \
>> + } while (0) \
>> +
>> DEFINE_PER_CPU(int, bpf_prog_active);
>> static DEFINE_IDR(prog_idr);
>> static DEFINE_SPINLOCK(prog_idr_lock);
>> @@ -1183,6 +1194,431 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
>> return err;
>> }
>>
>> +static void __map_batch_get_attrs(const union bpf_attr *attr,
>> + void __user **skey, void __user **nskey,
>> + void __user **keys, void __user **values,
>> + u32 *max_count, u64 *elem_flags, u64 *flags)
>> +{
>> + *skey = u64_to_user_ptr(attr->batch.start_key);
>> + *nskey = u64_to_user_ptr(attr->batch.next_start_key);
>> + *keys = u64_to_user_ptr(attr->batch.keys);
>> + *values = u64_to_user_ptr(attr->batch.values);
>> + *max_count = attr->batch.count;
>> + *elem_flags = attr->batch.elem_flags;
>> + *flags = attr->batch.flags;
>> +}
>> +
>> +static int
>> +__map_lookup_delete_batch_key_in_keys(struct bpf_map *map, void *key, void *value,
>> + u32 max_count, u32 key_size, u32 value_size,
>> + u64 elem_flags, void __user *keys,
>> + void __user *values,
>> + union bpf_attr __user *uattr,
>> + bool ignore_enoent)
>> +{
>> + u32 count, missed = 0;
>> + int ret = 0, err;
>> +
>> + for (count = 0; count < max_count; count++) {
>> + if (copy_from_user(key, keys + count * key_size, key_size)) {
>> + ret = -EFAULT;
>> + break;
>> + }
>> +
>> + ret = bpf_map_copy_value(map, key, value, elem_flags);
>> + if (ret) {
>> + if (ret != -ENOENT || !ignore_enoent)
>> + break;
>> +
>> + missed++;
>> + continue;
>> + }
>> +
>> +
>> + if (copy_to_user(values + count * value_size, value, value_size)) {
>> + ret = -EFAULT;
>> + break;
>> + }
>> +
>> + ret = bpf_map_delete_elem(map, key);
>> + if (ret) {
>> + if (ret != -ENOENT || !ignore_enoent)
>> + break;
>> +
>> + missed++;
>> + }
>> + }
>> +
>> + count -= missed;
>> + if ((!ret && missed) || (ret && ret != -EFAULT)) {
>> + err = put_user(count, &uattr->batch.count);
>> + ret = err ? : ret;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int map_lookup_and_delete_batch(struct bpf_map *map,
>> + const union bpf_attr *attr,
>> + union bpf_attr __user *uattr,
>> + bool do_delete)
>> +{
>> + u32 max_count, count = 0, key_size, roundup_key_size, value_size;
>> + bool ignore_enoent, nextkey_is_null, copied;
>> + void *buf = NULL, *key, *value, *next_key;
>> + void __user *skey, *nskey, *keys, *values;
>> + u64 elem_flags, flags, zero = 0;
>> + int err, ret = 0;
>> +
>> + if (map->map_type == BPF_MAP_TYPE_QUEUE ||
>> + map->map_type == BPF_MAP_TYPE_STACK)
>> + return -ENOTSUPP;
>> +
>> + __map_batch_get_attrs(attr, &skey, &nskey, &keys, &values, &max_count,
>> + &elem_flags, &flags);
>> +
>> + if (elem_flags & ~BPF_F_LOCK || flags & ~BPF_F_ENFORCE_ENOENT)
>> + return -EINVAL;
>> +
>> + if (!max_count)
>> + return 0;
>> +
>> + /* The following max_count/skey/nskey combinations are supported:
>> + * max_count != 0 && !skey && !nskey: loop/delete max_count elements in keys[]/values[].
>> + * max_count != 0 && !skey && nskey: loop/delete max_count elements starting from map start.
>> + * max_count != 0 && skey && nskey: loop/delete max_count elements starting from skey.
>> + */
>> + if (skey && !nskey)
>> + return -EINVAL;
>> +
>> + /* allocate space for two keys and one value. */
>> + key_size = map->key_size;
>> + roundup_key_size = round_up(map->key_size, 8);
>> + value_size = bpf_map_value_size(map);
>> + buf = kmalloc(roundup_key_size * 2 + value_size, GFP_USER | __GFP_NOWARN);
>> + if (!buf)
>> + return -ENOMEM;
>> +
>> + key = buf;
>> + next_key = buf + roundup_key_size;
>> + value = buf + roundup_key_size * 2;
>> + ignore_enoent = !(flags & BPF_F_ENFORCE_ENOENT);
>> +
>> + if (!skey && !nskey) {
>> + /* handle cases where keys in keys[] */
>> + ret = __map_lookup_delete_batch_key_in_keys(map, key, value, max_count,
>> + key_size, value_size,
>> + elem_flags, keys, values,
>> + uattr, ignore_enoent);
>> + goto out;
>> + }
>> +
>> + /* Get the first key. */
>> + if (!skey) {
>> + ret = bpf_map_get_next_key(map, NULL, key);
>> + if (ret) {
>> + nextkey_is_null = true;
>> + goto after_loop;
>> + }
>> + } else if (copy_from_user(key, skey, key_size)) {
>> + ret = -EFAULT;
>> + goto out;
>> + }
>> +
>> + /* Copy the first key/value pair */
>> + ret = bpf_map_copy_value(map, key, value, elem_flags);
>> + if (ret) {
>> + if (skey)
>> + goto out;
>> +
>> + nextkey_is_null = true;
>> + goto after_loop;
>> + }
>> +
>> + if (copy_to_user(keys, key, key_size) ||
>> + copy_to_user(values, value, value_size)) {
>> + ret = -EFAULT;
>> + goto out;
>> + }
>> +
>> + /* We will always try to get next_key first
>> + * and then delete the current key.
>> + */
>> + ret = bpf_map_get_next_key(map, key, next_key);
>
> One of the issues I see in this implementation is that is still
> relying on the existing functions and has the same consistency
> problems that my attempt had.
Right. The approach is very similar to what you proposed earlier.
>
> The problem happens when you are trying to do batch lookup on a
> hashmap and when executing bpf_map_get_next_key(map, key, next_key)
> the key is removed, then that call will return the first key and you'd
> start iterating the map from the beginning again and retrieve
> duplicate information.
Right. Maybe we can have another bpf_map_ops callback function
like 'map_batch_get_next_key' which won't fall back to the
first key if the 'key' is not available in the hash table?
If 'map_batch_get_next_key' failed due to 'key' is gone, we just
return to user space with whatever results we got so far.
We can have a flag to control this behavior. Some users may not
care about duplication.
>
> Note that sometimes you can also start from the same bucket when the
> key is updated while dumping it because it can be inserted on the head
> of the bucket so you could potentially revisit elements that you had
> already visited.
Do you think 'map_batch_get_next_key' approach could solve this as well?
>
> From previous discussion my understanding was that what we wanted was
> to pursue 'atomic' compounded operations first and after that, try to
> batch them. Although I don't think there's an easy way of batching and
> being consistent at the same time.
Yes, I thought about this 'atomic' compounded approach. First,
e.g., batch deletion, after some of elements are deleted, we found
one key is gone. we cannot really go back to the state where no
deletion happens.
Second, even we permit partial work, batched update/delete and element
update/delete concurrency (userspace or bpf program) still hard to
manage. We cannot put giant batched update/delete in a spin
lock... (bpf program uses spin lock for may update/delete operations).
I agree with you as I also did not find an easy way to have both
batching and good consistency.
^ permalink raw reply
* Re: [PATCH bpf-next 01/13] bpf: add bpf_map_value_size and bp_map_copy_value helper functions
From: Yonghong Song @ 2019-08-30 6:40 UTC (permalink / raw)
To: Song Liu
Cc: bpf, Networking, Alexei Starovoitov, Brian Vazquez,
Daniel Borkmann, Kernel Team
In-Reply-To: <2DB6B840-9EF6-483D-8570-4BB9EB74F3DA@fb.com>
On 8/29/19 3:04 PM, Song Liu wrote:
>
>
>> On Aug 28, 2019, at 11:45 PM, Yonghong Song <yhs@fb.com> wrote:
>>
>> From: Brian Vazquez <brianvv@google.com>
>>
>> Move reusable code from map_lookup_elem to helper functions to avoid code
>> duplication in kernel/bpf/syscall.c
>>
>> Suggested-by: Stanislav Fomichev <sdf@google.com>
>> Signed-off-by: Brian Vazquez <brianvv@google.com>
>
>
> Acked-by: Song Liu <songliubraving@fb.com>
>
> Yonghong, we also need your SoB.
Thanks for reminding me of this. Will add my SoB in next revision.
^ permalink raw reply
* Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
From: Greg Kroah-Hartman @ 2019-08-30 6:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jakub Kicinski, Julia Kartseva, ast, Thomas Gleixner, rdna, bpf,
daniel, netdev, kernel-team
In-Reply-To: <20190829171655.fww5qxtfusehcpds@ast-mbp.dhcp.thefacebook.com>
On Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov wrote:
> On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Aug 28, 2019 at 04:46:28PM -0700, Alexei Starovoitov wrote:
> > > On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
> > > >
> > > > Greg, Thomas, libbpf is extracted from the kernel sources and
> > > > maintained in a clone repo on GitHub for ease of packaging.
> > > >
> > > > IIUC Alexei's concern is that since we are moving the commits from
> > > > the kernel repo to the GitHub one we have to preserve the commits
> > > > exactly as they are, otherwise SOB lines lose their power.
> > > >
> > > > Can you provide some guidance on whether that's a valid concern,
> > > > or whether it's perfectly fine to apply a partial patch?
> > >
> > > Right. That's exactly the concern.
> > >
> > > Greg, Thomas,
> > > could you please put your legal hat on and clarify the following.
> > > Say some developer does a patch that modifies
> > > include/uapi/linux/bpf.h
> > > ..some other kernel code...and
> > > tools/include/uapi/linux/bpf.h
> > >
> > > That tools/include/uapi/linux/bpf.h is used by perf and by libbpf.
> > > We have automatic mirror of tools/libbpf into github/libbpf/
> > > so that external projects and can do git submodule of it,
> > > can build packages out of it, etc.
> > >
> > > The question is whether it's ok to split tools/* part out of
> > > original commit, keep Author and SOB, create new commit out of it,
> > > and automatically push that auto-generated commit into github mirror.
> >
> > Note, I am not a laywer, and am not _your_ lawyer either, only _your_
> > lawyer can answer questions as to what is best for you.
> >
> > That being said, from a "are you keeping the correct authorship info",
> > yes, it sounds like you are doing the correct thing here.
> >
> > Look at what I do for stable kernels, I take the original commit and add
> > it to "another tree" keeping the original author and s-o-b chain intact,
> > and adding a "this is the original git commit id" type message to the
> > changelog text so that people can link it back to the original.
>
> I think you're describing 'git cherry-pick -x'.
Well, my scripts don't use git, but yes, it's much the same thing :)
> The question was about taking pieces of the original commit. Not the whole commit.
> Author field obviously stays, but SOB is questionable.
sob matters to the file the commit is touching, and if it is identical
to the original file (including same license), then it should be fine.
> If author meant to change X and Y and Z. Silently taking only Z chunk of the diff
> doesn't quite seem right.
It can be confusing, I agree.
> If we document that such commit split happens in Documentation/bpf/bpf_devel_QA.rst
> do you think it will be enough to properly inform developers?
> The main concern is the surprise factor when people start seeing their commits
> in the mirror, but not their full commits.
Personally, I wouldn't care, but maybe you should just enforce the fact
that the original patch should ONLY touch Z, and not X and Y in the same
patch, to make this a lot more obvious.
Patches should only be doing "one logical thing" in the first place, but
maybe you also need to touch other things when doing a change that you
can't do this, I really do not know, sorry.
greg k-h
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Ivan Vecera @ 2019-08-30 6:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, idosch, andrew, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, allan.nielsen, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190830063624.GN2312@nanopsycho>
On Fri, 30 Aug 2019 08:36:24 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Aug 30, 2019 at 08:02:33AM CEST, davem@davemloft.net wrote:
> >From: Jiri Pirko <jiri@resnulli.us>
> >Date: Fri, 30 Aug 2019 07:39:40 +0200
> >
> >> Because the "promisc mode" would gain another meaning. Now how the
> >> driver should guess which meaning the user ment when he setted it?
> >> filter or trap?
> >>
> >> That is very confusing. If the flag is the way to do this, let's
> >> introduce another flag, like IFF_TRAPPING indicating that user
> >> wants exactly this.
> >
> >I don't understand how the meaning of promiscuous mode for a
> >networking device has suddenly become ambiguous, when did this start
> >happening?
>
> The promiscuity is a way to setup the rx filter. So promics == rx
> filter off. For normal nics, where there is no hw fwd datapath,
> this coincidentally means all received packets go to cpu.
> But if there is hw fwd datapath, rx filter is still off, all rxed
> packets are processed. But that does not mean they should be trapped
> to cpu.
+1
Promisc is Rx filtering option and should not imply that offloaded
traffic is trapped to CPU.
> Simple example:
> I need to see slowpath packets, for example arps/stp/bgp/... that
> are going to cpu, I do:
> tcpdump -i swp1
>
> I don't want to get all the traffic running over hw running this cmd.
> This is a valid usecase.
>
> To cope with hw fwd datapath devices, I believe that tcpdump has to
> have notion of that. Something like:
>
> tcpdump -i swp1 --hw-trapping-mode
>
> The logic can be inverse:
> tcpdump -i swp1
> tcpdump -i swp1 --no-hw-trapping-mode
>
> However, that would provide inconsistent behaviour between existing
> and patched tcpdump/kernel.
>
> All I'm trying to say, there are 2 flags
> needed (if we don't use tc trap).
^ permalink raw reply
* Re: [PATCH bpf-next 05/13] bpf: adding map batch processing support
From: Alexei Starovoitov @ 2019-08-30 6:58 UTC (permalink / raw)
To: Yonghong Song
Cc: Brian Vazquez, bpf@vger.kernel.org, netdev@vger.kernel.org,
Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <2581bf69-2e35-fb10-7b84-3869286f6c85@fb.com>
On Fri, Aug 30, 2019 at 06:39:48AM +0000, Yonghong Song wrote:
> >
> > The problem happens when you are trying to do batch lookup on a
> > hashmap and when executing bpf_map_get_next_key(map, key, next_key)
> > the key is removed, then that call will return the first key and you'd
> > start iterating the map from the beginning again and retrieve
> > duplicate information.
>
> Right. Maybe we can have another bpf_map_ops callback function
> like 'map_batch_get_next_key' which won't fall back to the
> first key if the 'key' is not available in the hash table?
The reason I picked this get_next_key behavior long ago
because I couldn't come up with a way to pick the next key consistently.
In the hash table the elements are not sorted.
If there are more than one element in the hash table bucket
they are added to the link list in sort-of random order.
If one out of N elems in the bucket are deleted which one should be
picked next?
select_bucket() picks the bucket.
if lookup_nulls_elem_raw() cannot find the element which one in
the link list is the "right one" to continue?
Iterating over hash table without duplicates when elements
are being added and removed in parallel is a hard problem to solve.
I think "best effort" is the right answer.
When users care about consistency they should use map-in-map.
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: David Miller @ 2019-08-30 7:12 UTC (permalink / raw)
To: jiri
Cc: idosch, andrew, horatiu.vultur, alexandre.belloni, UNGLinuxDriver,
allan.nielsen, ivecera, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190830063624.GN2312@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 30 Aug 2019 08:36:24 +0200
> The promiscuity is a way to setup the rx filter. So promics == rx filter
> off. For normal nics, where there is no hw fwd datapath,
> this coincidentally means all received packets go to cpu.
You cannot convince me that the HW datapath isn't a "rx filter" too, sorry.
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: David Miller @ 2019-08-30 7:13 UTC (permalink / raw)
To: ivecera
Cc: jiri, idosch, andrew, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, allan.nielsen, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190830085445.1c28dc02@ceranb>
From: Ivan Vecera <ivecera@redhat.com>
Date: Fri, 30 Aug 2019 08:54:45 +0200
> Promisc is Rx filtering option and should not imply that offloaded
> traffic is trapped to CPU.
Hardware is offloaded based upon an rx filter.
Stop this nonsense.
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Jiri Pirko @ 2019-08-30 7:21 UTC (permalink / raw)
To: David Miller
Cc: idosch, andrew, horatiu.vultur, alexandre.belloni, UNGLinuxDriver,
allan.nielsen, ivecera, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190830.001223.669650763835949848.davem@davemloft.net>
Fri, Aug 30, 2019 at 09:12:23AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri, 30 Aug 2019 08:36:24 +0200
>
>> The promiscuity is a way to setup the rx filter. So promics == rx filter
>> off. For normal nics, where there is no hw fwd datapath,
>> this coincidentally means all received packets go to cpu.
>
>You cannot convince me that the HW datapath isn't a "rx filter" too, sorry.
If you look at it that way, then we have 2: rx_filter and hw_rx_filter.
The point is, those 2 are not one item, that is the point I'm trying to
make :/
^ permalink raw reply
* [PATCH 0/2] pull request for net: batman-adv 2019-08-30
From: Simon Wunderlich @ 2019-08-30 7:25 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Simon Wunderlich
Hi David,
here is another small batman-adv pull request for net.
Please pull or let me know of any problem!
Thank you,
Simon
The following changes since commit 3ee1bb7aae97324ec9078da1f00cb2176919563f:
batman-adv: fix uninit-value in batadv_netlink_get_ifindex() (2019-08-14 19:27:07 +0200)
are available in the Git repository at:
git://git.open-mesh.org/linux-merge.git tags/batadv-net-for-davem-20190830
for you to fetch changes up to 0ff0f15a32c093381ad1abc06abe85afb561ab28:
batman-adv: Only read OGM2 tvlv_len after buffer len check (2019-08-23 18:20:31 +0200)
----------------------------------------------------------------
Here are two batman-adv bugfixes:
- Fix OGM and OGMv2 header read boundary check,
by Sven Eckelmann (2 patches)
----------------------------------------------------------------
Sven Eckelmann (2):
batman-adv: Only read OGM tvlv_len after buffer len check
batman-adv: Only read OGM2 tvlv_len after buffer len check
net/batman-adv/bat_iv_ogm.c | 20 +++++++++++++-------
net/batman-adv/bat_v_ogm.c | 18 ++++++++++++------
2 files changed, 25 insertions(+), 13 deletions(-)
^ permalink raw reply
* [PATCH 1/2] batman-adv: Only read OGM tvlv_len after buffer len check
From: Simon Wunderlich @ 2019-08-30 7:25 UTC (permalink / raw)
To: davem
Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, syzbot+355cab184197dbbfa384,
Antonio Quartulli, Simon Wunderlich
In-Reply-To: <20190830072502.16929-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
Multiple batadv_ogm_packet can be stored in an skbuff. The functions
batadv_iv_ogm_send_to_if()/batadv_iv_ogm_receive() use
batadv_iv_ogm_aggr_packet() to check if there is another additional
batadv_ogm_packet in the skb or not before they continue processing the
packet.
The length for such an OGM is BATADV_OGM_HLEN +
batadv_ogm_packet->tvlv_len. The check must first check that at least
BATADV_OGM_HLEN bytes are available before it accesses tvlv_len (which is
part of the header. Otherwise it might try read outside of the currently
available skbuff to get the content of tvlv_len.
Fixes: ef26157747d4 ("batman-adv: tvlv - basic infrastructure")
Reported-by: syzbot+355cab184197dbbfa384@syzkaller.appspotmail.com
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Acked-by: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_iv_ogm.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 240ed70912d6..d78938e3e008 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -277,17 +277,23 @@ static u8 batadv_hop_penalty(u8 tq, const struct batadv_priv *bat_priv)
* batadv_iv_ogm_aggr_packet() - checks if there is another OGM attached
* @buff_pos: current position in the skb
* @packet_len: total length of the skb
- * @tvlv_len: tvlv length of the previously considered OGM
+ * @ogm_packet: potential OGM in buffer
*
* Return: true if there is enough space for another OGM, false otherwise.
*/
-static bool batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len,
- __be16 tvlv_len)
+static bool
+batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len,
+ const struct batadv_ogm_packet *ogm_packet)
{
int next_buff_pos = 0;
- next_buff_pos += buff_pos + BATADV_OGM_HLEN;
- next_buff_pos += ntohs(tvlv_len);
+ /* check if there is enough space for the header */
+ next_buff_pos += buff_pos + sizeof(*ogm_packet);
+ if (next_buff_pos > packet_len)
+ return false;
+
+ /* check if there is enough space for the optional TVLV */
+ next_buff_pos += ntohs(ogm_packet->tvlv_len);
return (next_buff_pos <= packet_len) &&
(next_buff_pos <= BATADV_MAX_AGGREGATION_BYTES);
@@ -315,7 +321,7 @@ static void batadv_iv_ogm_send_to_if(struct batadv_forw_packet *forw_packet,
/* adjust all flags and log packets */
while (batadv_iv_ogm_aggr_packet(buff_pos, forw_packet->packet_len,
- batadv_ogm_packet->tvlv_len)) {
+ batadv_ogm_packet)) {
/* we might have aggregated direct link packets with an
* ordinary base packet
*/
@@ -1704,7 +1710,7 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
/* unpack the aggregated packets and process them one by one */
while (batadv_iv_ogm_aggr_packet(ogm_offset, skb_headlen(skb),
- ogm_packet->tvlv_len)) {
+ ogm_packet)) {
batadv_iv_ogm_process(skb, ogm_offset, if_incoming);
ogm_offset += BATADV_OGM_HLEN;
--
2.20.1
^ permalink raw reply related
* [PATCH 2/2] batman-adv: Only read OGM2 tvlv_len after buffer len check
From: Simon Wunderlich @ 2019-08-30 7:25 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, Simon Wunderlich
In-Reply-To: <20190830072502.16929-1-sw@simonwunderlich.de>
From: Sven Eckelmann <sven@narfation.org>
Multiple batadv_ogm2_packet can be stored in an skbuff. The functions
batadv_v_ogm_send_to_if() uses batadv_v_ogm_aggr_packet() to check if there
is another additional batadv_ogm2_packet in the skb or not before they
continue processing the packet.
The length for such an OGM2 is BATADV_OGM2_HLEN +
batadv_ogm2_packet->tvlv_len. The check must first check that at least
BATADV_OGM2_HLEN bytes are available before it accesses tvlv_len (which is
part of the header. Otherwise it might try read outside of the currently
available skbuff to get the content of tvlv_len.
Fixes: 9323158ef9f4 ("batman-adv: OGMv2 - implement originators logic")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_v_ogm.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index fad95ef64e01..bc06e3cdfa84 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -631,17 +631,23 @@ batadv_v_ogm_process_per_outif(struct batadv_priv *bat_priv,
* batadv_v_ogm_aggr_packet() - checks if there is another OGM aggregated
* @buff_pos: current position in the skb
* @packet_len: total length of the skb
- * @tvlv_len: tvlv length of the previously considered OGM
+ * @ogm2_packet: potential OGM2 in buffer
*
* Return: true if there is enough space for another OGM, false otherwise.
*/
-static bool batadv_v_ogm_aggr_packet(int buff_pos, int packet_len,
- __be16 tvlv_len)
+static bool
+batadv_v_ogm_aggr_packet(int buff_pos, int packet_len,
+ const struct batadv_ogm2_packet *ogm2_packet)
{
int next_buff_pos = 0;
- next_buff_pos += buff_pos + BATADV_OGM2_HLEN;
- next_buff_pos += ntohs(tvlv_len);
+ /* check if there is enough space for the header */
+ next_buff_pos += buff_pos + sizeof(*ogm2_packet);
+ if (next_buff_pos > packet_len)
+ return false;
+
+ /* check if there is enough space for the optional TVLV */
+ next_buff_pos += ntohs(ogm2_packet->tvlv_len);
return (next_buff_pos <= packet_len) &&
(next_buff_pos <= BATADV_MAX_AGGREGATION_BYTES);
@@ -818,7 +824,7 @@ int batadv_v_ogm_packet_recv(struct sk_buff *skb,
ogm_packet = (struct batadv_ogm2_packet *)skb->data;
while (batadv_v_ogm_aggr_packet(ogm_offset, skb_headlen(skb),
- ogm_packet->tvlv_len)) {
+ ogm_packet)) {
batadv_v_ogm_process(skb, ogm_offset, if_incoming);
ogm_offset += BATADV_OGM2_HLEN;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next 00/13] bpf: adding map batch processing support
From: Yonghong Song @ 2019-08-30 7:25 UTC (permalink / raw)
To: Jakub Kicinski, Alexei Starovoitov
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org, Brian Vazquez,
Daniel Borkmann, Kernel Team
In-Reply-To: <20190829113932.5c058194@cakuba.netronome.com>
On 8/29/19 11:39 AM, Jakub Kicinski wrote:
> On Wed, 28 Aug 2019 23:45:02 -0700, Yonghong Song wrote:
>> Brian Vazquez has proposed BPF_MAP_DUMP command to look up more than one
>> map entries per syscall.
>> https://lore.kernel.org/bpf/CABCgpaU3xxX6CMMxD+1knApivtc2jLBHysDXw-0E9bQEL0qC3A@mail.gmail.com/T/#t
>>
>> During discussion, we found more use cases can be supported in a similar
>> map operation batching framework. For example, batched map lookup and delete,
>> which can be really helpful for bcc.
>> https://github.com/iovisor/bcc/blob/master/tools/tcptop.py#L233-L243
>> https://github.com/iovisor/bcc/blob/master/tools/slabratetop.py#L129-L138
>>
>> Also, in bcc, we have API to delete all entries in a map.
>> https://github.com/iovisor/bcc/blob/master/src/cc/api/BPFTable.h#L257-L264
>>
>> For map update, batched operations also useful as sometimes applications need
>> to populate initial maps with more than one entry. For example, the below
>> example is from kernel/samples/bpf/xdp_redirect_cpu_user.c:
>> https://github.com/torvalds/linux/blob/master/samples/bpf/xdp_redirect_cpu_user.c#L543-L550
>>
>> This patch addresses all the above use cases. To make uapi stable, it also
>> covers other potential use cases. Four bpf syscall subcommands are introduced:
>> BPF_MAP_LOOKUP_BATCH
>> BPF_MAP_LOOKUP_AND_DELETE_BATCH
>> BPF_MAP_UPDATE_BATCH
>> BPF_MAP_DELETE_BATCH
>>
>> In userspace, application can iterate through the whole map one batch
>> as a time, e.g., bpf_map_lookup_batch() in the below:
>> p_key = NULL;
>> p_next_key = &key;
>> while (true) {
>> err = bpf_map_lookup_batch(fd, p_key, &p_next_key, keys, values,
>> &batch_size, elem_flags, flags);
>> if (err) ...
>> if (p_next_key) break; // done
>> if (!p_key) p_key = p_next_key;
>> }
>> Please look at individual patches for details of new syscall subcommands
>> and examples of user codes.
>>
>> The testing is also done in a qemu VM environment:
>> measure_lookup: max_entries 1000000, batch 10, time 342ms
>> measure_lookup: max_entries 1000000, batch 1000, time 295ms
>> measure_lookup: max_entries 1000000, batch 1000000, time 270ms
>> measure_lookup: max_entries 1000000, no batching, time 1346ms
>> measure_lookup_delete: max_entries 1000000, batch 10, time 433ms
>> measure_lookup_delete: max_entries 1000000, batch 1000, time 363ms
>> measure_lookup_delete: max_entries 1000000, batch 1000000, time 357ms
>> measure_lookup_delete: max_entries 1000000, not batch, time 1894ms
>> measure_delete: max_entries 1000000, batch, time 220ms
>> measure_delete: max_entries 1000000, not batch, time 1289ms
>> For a 1M entry hash table, batch size of 10 can reduce cpu time
>> by 70%. Please see patch "tools/bpf: measure map batching perf"
>> for details of test codes.
>
> Hi Yonghong!
>
> great to see this, we have been looking at implementing some way to
> speed up map walks as well.
>
> The direction we were looking in, after previous discussions [1],
> however, was to provide a BPF program which can run the logic entirely
> within the kernel.
>
> We have a rough PoC on the FW side (we can offload the program which
> walks the map, which is pretty neat), but the kernel verifier side
> hasn't really progressed. It will soon.
>
> The rough idea is that the user space provides two programs, "filter"
> and "dumper":
>
> bpftool map exec id XYZ filter pinned /some/prog \
> dumper pinned /some/other_prog
>
> Both programs get this context:
>
> struct map_op_ctx {
> u64 key;
> u64 value;
> }
>
> We need a per-map implementation of the exec side, but roughly maps
> would do:
>
> LIST_HEAD(deleted);
>
> for entry in map {
> struct map_op_ctx {
> .key = entry->key,
> .value = entry->value,
> };
>
> act = BPF_PROG_RUN(filter, &map_op_ctx);
> if (act & ~ACT_BITS)
> return -EINVAL;
>
> if (act & DELETE) {
> map_unlink(entry);
> list_add(entry, &deleted);
> }
> if (act & STOP)
> break;
> }
>
> synchronize_rcu();
>
> for entry in deleted {
> struct map_op_ctx {
> .key = entry->key,
> .value = entry->value,
> };
>
> BPF_PROG_RUN(dumper, &map_op_ctx);
> map_free(entry);
> }
>
> The filter program can't perform any map operations other than lookup,
> otherwise we won't be able to guarantee that we'll walk the entire map
> (if the filter program deletes some entries in a unfortunate order).
Looks like you will provide a new program type and per-map
implementation of above code. My patch set indeed avoided per-map
implementation for all of lookup/delete/get-next-key...
>
> If user space just wants a pure dump it can simply load a program which
> dumps the entries into a perf ring.
percpu perf ring is not really ideal for user space which simply just
want to get some key/value pairs back. Some kind of generate non-per-cpu
ring buffer might be better for such cases.
>
> I'm bringing this up because that mechanism should cover what is
> achieved with this patch set and much more.
The only case it did not cover is batched update. But that may not
be super critical.
Your approach give each element an action choice through another bpf
program. This indeed powerful. My use case is simpler than your use case
below, hence the implementation.
>
> In particular for networking workloads where old flows have to be
> pruned from the map periodically it's far more efficient to communicate
> to user space only the flows which timed out (the delete batching from
> this set won't help at all).
Maybe LRU map will help in this case? It is designed for such
use cases.
>
> With a 2M entry map and this patch set we still won't be able to prune
> once a second on one core.
>
> [1]
> https://lore.kernel.org/netdev/20190813130921.10704-4-quentin.monnet@netronome.com/
>
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Ivan Vecera @ 2019-08-30 7:26 UTC (permalink / raw)
To: Jiri Pirko
Cc: Andrew Lunn, Horatiu Vultur, alexandre.belloni, UNGLinuxDriver,
davem, allan.nielsen, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190830061327.GM2312@nanopsycho>
On Fri, 30 Aug 2019 08:13:27 +0200
Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Aug 29, 2019 at 04:37:32PM CEST, andrew@lunn.ch wrote:
> >> Wait, I believe there has been some misundestanding. Promisc mode
> >> is NOT about getting packets to the cpu. It's about setting hw
> >> filters in a way that no rx packet is dropped.
> >>
> >> If you want to get packets from the hw forwarding dataplane to
> >> cpu, you should not use promisc mode for that. That would be
> >> incorrect.
> >
> >Hi Jiri
> >
> >I'm not sure a wireshark/tcpdump/pcap user would agree with you. They
> >want to see packets on an interface, so they use these tools. The
> >fact that the interface is a switch interface should not matter. The
> >switchdev model is that we try to hide away the interface happens to
> >be on a switch, you can just use it as normal. So why should promisc
> >mode not work as normal?
>
> It does, disables the rx filter. Why do you think it means the same
> thing as "trap all to cpu"? Hw datapath was never considered by
> wireshark.
>
> In fact, I have usecase where I need to see only slow-path traffic by
> wireshark, not all packets going through hw. So apparently, there is a
> need of another wireshark option and perhaps another flag
> IFF_HW_TRAPPING?.
Agree with Jiri but understand both perspectives. We can treat
IFF_PROMISC as:
1) "I want to _SEE_ all Rx traffic on specified interface"
that means for switchdev driver that it has to trap all traffic to CPU
implicitly. And in this case we need another flag that will say "I
don't want to see offloaded traffic".
2) "I want to ensure that nothing is dropped on specified interface" so
IFF_PROMISC is treated as filtering option only. To see offloaded
traffic you need to setup TC rule with trap action or another flag like
IFF_TRAPPING.
IMO IFF_PROMISC should be considered to be a filtering option and
should not imply trapping of offloaded traffic.
Thanks,
Ivan
^ permalink raw reply
* [PATCH 0/1] pull request for net-next: batman-adv 2019-08-30
From: Simon Wunderlich @ 2019-08-30 7:27 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Simon Wunderlich
Hi David,
here is a small maintenance pull request of batman-adv to go into net-next.
Please pull or let me know of any problem!
Thank you,
Simon
The following changes since commit 9cb9a17813bf0de1f8ad6deb9538296d5148b5a8:
batman-adv: BATMAN_V: aggregate OGMv2 packets (2019-08-04 22:22:00 +0200)
are available in the Git repository at:
git://git.open-mesh.org/linux-merge.git tags/batadv-next-for-davem-20190830
for you to fetch changes up to 2a813f1392205654aea28098f3bcc3e6e2478fa5:
batman-adv: Add Sven to MAINTAINERS file (2019-08-17 13:11:50 +0200)
----------------------------------------------------------------
This maintenance patchset includes the following patches:
- Add Sven to the MAINTAINERS file, by Simon Wunderlich
----------------------------------------------------------------
Simon Wunderlich (1):
batman-adv: Add Sven to MAINTAINERS file
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
^ permalink raw reply
* [PATCH 1/1] batman-adv: Add Sven to MAINTAINERS file
From: Simon Wunderlich @ 2019-08-30 7:27 UTC (permalink / raw)
To: davem; +Cc: netdev, b.a.t.m.a.n, Simon Wunderlich, Antonio Quartulli
In-Reply-To: <20190830072736.18535-1-sw@simonwunderlich.de>
Sven is taking care of tracking our patches and merging most of them in
our tree. Let's add him to the MAINTAINERS file so he will get all
patch e-mails.
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
Acked-by: Antonio Quartulli <a@unstable.cc>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 783569e3c4b4..ce8316cbe3b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2911,6 +2911,7 @@ BATMAN ADVANCED
M: Marek Lindner <mareklindner@neomailbox.ch>
M: Simon Wunderlich <sw@simonwunderlich.de>
M: Antonio Quartulli <a@unstable.cc>
+M: Sven Eckelmann <sven@narfation.org>
L: b.a.t.m.a.n@lists.open-mesh.org (moderated for non-subscribers)
W: https://www.open-mesh.org/
B: https://www.open-mesh.org/projects/batman-adv/issues
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: David Miller @ 2019-08-30 7:32 UTC (permalink / raw)
To: jiri
Cc: idosch, andrew, horatiu.vultur, alexandre.belloni, UNGLinuxDriver,
allan.nielsen, ivecera, f.fainelli, netdev, linux-kernel
In-Reply-To: <20190830072133.GP2312@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 30 Aug 2019 09:21:33 +0200
> Fri, Aug 30, 2019 at 09:12:23AM CEST, davem@davemloft.net wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Date: Fri, 30 Aug 2019 08:36:24 +0200
>>
>>> The promiscuity is a way to setup the rx filter. So promics == rx filter
>>> off. For normal nics, where there is no hw fwd datapath,
>>> this coincidentally means all received packets go to cpu.
>>
>>You cannot convince me that the HW datapath isn't a "rx filter" too, sorry.
>
> If you look at it that way, then we have 2: rx_filter and hw_rx_filter.
> The point is, those 2 are not one item, that is the point I'm trying to
> make :/
And you can turn both of them off when I ask for promiscuous mode, that's
a detail of the device not a semantic issue.
^ permalink raw reply
* [PATCH net-next 0/4] qed*: Enhancements.
From: Sudarsana Reddy Kalluru @ 2019-08-30 7:42 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
The patch series adds couple of enhancements to qed/qede drivers.
- Support for dumping the config id attributes via ethtool -w/W.
- Support for dumping the GRC data of required memory regions using
ethtool -w/W interfaces.
Patch (1) adds driver APIs for reading the config id attributes.
Patch (2) adds ethtool support for dumping the config id attributes.
Patch (3) adds support for configuring the GRC dump config flags.
Patch (4) adds ethtool support for dumping the grc dump.
Please consider applying it to net-next.
Sudarsana Reddy Kalluru (4):
qed: Add APIs for reading config id attributes.
qede: Add support for reading the config id attributes.
qed: Add APIs for configuring grc dump config flags.
qede: Add support for dumping the grc data.
drivers/net/ethernet/qlogic/qed/qed_debug.c | 82 +++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 15 ++++
drivers/net/ethernet/qlogic/qed/qed_main.c | 48 ++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 29 ++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 15 ++++
drivers/net/ethernet/qlogic/qede/qede.h | 15 ++++
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 114 ++++++++++++++++++++++++
include/linux/qed/qed_if.h | 20 +++++
8 files changed, 338 insertions(+)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 1/4] qed: Add APIs for reading config id attributes.
From: Sudarsana Reddy Kalluru @ 2019-08-30 7:42 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190830074206.8836-1-skalluru@marvell.com>
The patch adds driver support for reading the config id attributes from NVM
flash partition.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qed/qed_main.c | 27 +++++++++++++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 29 +++++++++++++++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 15 +++++++++++++++
include/linux/qed/qed_if.h | 11 +++++++++++
4 files changed, 82 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 7891f8c..c9a7571 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -69,6 +69,8 @@
#define QED_RDMA_SRQS QED_ROCE_QPS
#define QED_NVM_CFG_SET_FLAGS 0xE
#define QED_NVM_CFG_SET_PF_FLAGS 0x1E
+#define QED_NVM_CFG_GET_FLAGS 0xA
+#define QED_NVM_CFG_GET_PF_FLAGS 0x1A
static char version[] =
"QLogic FastLinQ 4xxxx Core Module qed " DRV_MODULE_VERSION "\n";
@@ -2298,6 +2300,30 @@ static int qed_nvm_flash_cfg_write(struct qed_dev *cdev, const u8 **data)
return rc;
}
+static int qed_nvm_flash_cfg_read(struct qed_dev *cdev, u8 **data,
+ u32 cmd, u32 entity_id)
+{
+ struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
+ struct qed_ptt *ptt;
+ u32 flags, len;
+ int rc = 0;
+
+ ptt = qed_ptt_acquire(hwfn);
+ if (!ptt)
+ return -EAGAIN;
+
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "Read config cmd = %d entity id %d\n", cmd, entity_id);
+ flags = entity_id ? QED_NVM_CFG_GET_PF_FLAGS : QED_NVM_CFG_GET_FLAGS;
+ rc = qed_mcp_nvm_get_cfg(hwfn, ptt, cmd, entity_id, flags, *data, &len);
+ if (rc)
+ DP_ERR(cdev, "Error %d reading %d\n", rc, cmd);
+
+ qed_ptt_release(hwfn, ptt);
+
+ return rc;
+}
+
static int qed_nvm_flash(struct qed_dev *cdev, const char *name)
{
const struct firmware *image;
@@ -2610,6 +2636,7 @@ static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
.db_recovery_del = &qed_db_recovery_del,
.read_module_eeprom = &qed_read_module_eeprom,
.get_affin_hwfn_idx = &qed_get_affin_hwfn_idx,
+ .read_nvm_cfg = &qed_nvm_flash_cfg_read,
};
void qed_get_protocol_stats(struct qed_dev *cdev,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 89462c4..36ddb89 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -3751,6 +3751,35 @@ int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
return 0;
}
+int qed_mcp_nvm_get_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+ u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+ u32 *p_len)
+{
+ u32 mb_param = 0, resp, param;
+ int rc;
+
+ QED_MFW_SET_FIELD(mb_param, DRV_MB_PARAM_NVM_CFG_OPTION_ID, option_id);
+ if (flags & QED_NVM_CFG_OPTION_INIT)
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_INIT, 1);
+ if (flags & QED_NVM_CFG_OPTION_FREE)
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_FREE, 1);
+ if (flags & QED_NVM_CFG_OPTION_ENTITY_SEL) {
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL, 1);
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID,
+ entity_id);
+ }
+
+ rc = qed_mcp_nvm_rd_cmd(p_hwfn, p_ptt,
+ DRV_MSG_CODE_GET_NVM_CFG_OPTION,
+ mb_param, &resp, ¶m, p_len, (u32 *)p_buf);
+
+ return rc;
+}
+
int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
u32 len)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index 83649a8..9c4c276 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -1209,6 +1209,21 @@ void qed_mcp_resc_lock_default_init(struct qed_resc_lock_params *p_lock,
int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
/**
+ * @brief Get NVM config attribute value.
+ *
+ * @param p_hwfn
+ * @param p_ptt
+ * @param option_id
+ * @param entity_id
+ * @param flags
+ * @param p_buf
+ * @param p_len
+ */
+int qed_mcp_nvm_get_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+ u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+ u32 *p_len);
+
+/**
* @brief Set NVM config attribute value.
*
* @param p_hwfn
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index e366399..06fd958 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -1132,6 +1132,17 @@ struct qed_common_ops {
* @param cdev
*/
u8 (*get_affin_hwfn_idx)(struct qed_dev *cdev);
+
+/**
+ * @brief read_nvm_cfg - Read NVM config attribute value.
+ * @param cdev
+ * @param buf - buffer
+ * @param cmd - NVM CFG command id
+ * @param entity_id - Entity id
+ *
+ */
+ int (*read_nvm_cfg)(struct qed_dev *cdev, u8 **buf, u32 cmd,
+ u32 entity_id);
};
#define MASK_FIELD(_name, _value) \
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 2/4] qede: Add support for reading the config id attributes.
From: Sudarsana Reddy Kalluru @ 2019-08-30 7:42 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190830074206.8836-1-skalluru@marvell.com>
Add driver support for dumping the config id attributes via ethtool dump
interfaces.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qede/qede.h | 14 ++++
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 89 +++++++++++++++++++++++++
2 files changed, 103 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index 0e931c0..8f2adde 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -177,6 +177,19 @@ enum qede_flags_bit {
QEDE_FLAGS_TX_TIMESTAMPING_EN
};
+#define QEDE_DUMP_MAX_ARGS 4
+enum qede_dump_cmd {
+ QEDE_DUMP_CMD_NONE = 0,
+ QEDE_DUMP_CMD_NVM_CFG,
+ QEDE_DUMP_CMD_MAX
+};
+
+struct qede_dump_info {
+ enum qede_dump_cmd cmd;
+ u8 num_args;
+ u32 args[QEDE_DUMP_MAX_ARGS];
+};
+
struct qede_dev {
struct qed_dev *cdev;
struct net_device *ndev;
@@ -262,6 +275,7 @@ struct qede_dev {
struct qede_rdma_dev rdma_info;
struct bpf_prog *xdp_prog;
+ struct qede_dump_info dump_info;
};
enum QEDE_STATE {
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index abcee47..2359293 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -48,6 +48,9 @@
{QEDE_RQSTAT_OFFSET(stat_name), QEDE_RQSTAT_STRING(stat_name)}
#define QEDE_SELFTEST_POLL_COUNT 100
+#define QEDE_DUMP_VERSION 0x1
+#define QEDE_DUMP_NVM_BUF_LEN 32
+#define QEDE_DUMP_NVM_ARG_COUNT 2
static const struct {
u64 offset;
@@ -1973,6 +1976,89 @@ static int qede_get_module_eeprom(struct net_device *dev,
return rc;
}
+static int qede_set_dump(struct net_device *dev, struct ethtool_dump *val)
+{
+ struct qede_dev *edev = netdev_priv(dev);
+ int rc = 0;
+
+ if (edev->dump_info.cmd == QEDE_DUMP_CMD_NONE) {
+ if (val->flag > QEDE_DUMP_CMD_MAX) {
+ DP_ERR(edev, "Invalid command %d\n", val->flag);
+ return -EINVAL;
+ }
+ edev->dump_info.cmd = val->flag;
+ edev->dump_info.num_args = 0;
+ return 0;
+ }
+
+ if (edev->dump_info.num_args == QEDE_DUMP_MAX_ARGS) {
+ DP_ERR(edev, "Arg count = %d\n", edev->dump_info.num_args);
+ return -EINVAL;
+ }
+
+ switch (edev->dump_info.cmd) {
+ case QEDE_DUMP_CMD_NVM_CFG:
+ edev->dump_info.args[edev->dump_info.num_args] = val->flag;
+ edev->dump_info.num_args++;
+ break;
+ default:
+ break;
+ }
+
+ return rc;
+}
+
+static int qede_get_dump_flag(struct net_device *dev,
+ struct ethtool_dump *dump)
+{
+ struct qede_dev *edev = netdev_priv(dev);
+
+ dump->version = QEDE_DUMP_VERSION;
+ switch (edev->dump_info.cmd) {
+ case QEDE_DUMP_CMD_NVM_CFG:
+ dump->flag = QEDE_DUMP_CMD_NVM_CFG;
+ dump->len = QEDE_DUMP_NVM_BUF_LEN;
+ break;
+ default:
+ break;
+ }
+
+ DP_VERBOSE(edev, QED_MSG_DEBUG,
+ "dump->version = 0x%x dump->flag = %d dump->len = %d\n",
+ dump->version, dump->flag, dump->len);
+ return 0;
+}
+
+static int qede_get_dump_data(struct net_device *dev,
+ struct ethtool_dump *dump, void *buf)
+{
+ struct qede_dev *edev = netdev_priv(dev);
+ int rc;
+
+ switch (edev->dump_info.cmd) {
+ case QEDE_DUMP_CMD_NVM_CFG:
+ if (edev->dump_info.num_args != QEDE_DUMP_NVM_ARG_COUNT) {
+ DP_ERR(edev, "Arg count = %d required = %d\n",
+ edev->dump_info.num_args,
+ QEDE_DUMP_NVM_ARG_COUNT);
+ return -EINVAL;
+ }
+ rc = edev->ops->common->read_nvm_cfg(edev->cdev, (u8 **)&buf,
+ edev->dump_info.args[0],
+ edev->dump_info.args[1]);
+ break;
+ default:
+ DP_ERR(edev, "Invalid cmd = %d\n", edev->dump_info.cmd);
+ rc = -EINVAL;
+ break;
+ }
+
+ edev->dump_info.cmd = QEDE_DUMP_CMD_NONE;
+ edev->dump_info.num_args = 0;
+
+ return rc;
+}
+
static const struct ethtool_ops qede_ethtool_ops = {
.get_link_ksettings = qede_get_link_ksettings,
.set_link_ksettings = qede_set_link_ksettings,
@@ -2014,6 +2100,9 @@ static int qede_get_module_eeprom(struct net_device *dev,
.get_tunable = qede_get_tunable,
.set_tunable = qede_set_tunable,
.flash_device = qede_flash_device,
+ .get_dump_flag = qede_get_dump_flag,
+ .get_dump_data = qede_get_dump_data,
+ .set_dump = qede_set_dump,
};
static const struct ethtool_ops qede_vf_ethtool_ops = {
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/4] qed: Add APIs for configuring grc dump config flags.
From: Sudarsana Reddy Kalluru @ 2019-08-30 7:42 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190830074206.8836-1-skalluru@marvell.com>
The patch adds driver support for configuring the grc dump config flags.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qed/qed_debug.c | 82 +++++++++++++++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 15 ++++++
drivers/net/ethernet/qlogic/qed/qed_main.c | 21 ++++++++
include/linux/qed/qed_if.h | 9 ++++
4 files changed, 127 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_debug.c b/drivers/net/ethernet/qlogic/qed/qed_debug.c
index 5ea6c4f..859caa6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_debug.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_debug.c
@@ -1756,6 +1756,15 @@ static u32 qed_read_unaligned_dword(u8 *buf)
return dword;
}
+/* Sets the value of the specified GRC param */
+static void qed_grc_set_param(struct qed_hwfn *p_hwfn,
+ enum dbg_grc_params grc_param, u32 val)
+{
+ struct dbg_tools_data *dev_data = &p_hwfn->dbg_info;
+
+ dev_data->grc.param_val[grc_param] = val;
+}
+
/* Returns the value of the specified GRC param */
static u32 qed_grc_get_param(struct qed_hwfn *p_hwfn,
enum dbg_grc_params grc_param)
@@ -5119,6 +5128,69 @@ bool qed_read_fw_info(struct qed_hwfn *p_hwfn,
return false;
}
+enum dbg_status qed_dbg_grc_config(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ enum dbg_grc_params grc_param, u32 val)
+{
+ enum dbg_status status;
+ int i;
+
+ DP_VERBOSE(p_hwfn, QED_MSG_DEBUG,
+ "dbg_grc_config: paramId = %d, val = %d\n", grc_param, val);
+
+ status = qed_dbg_dev_init(p_hwfn, p_ptt);
+ if (status != DBG_STATUS_OK)
+ return status;
+
+ /* Initializes the GRC parameters (if not initialized). Needed in order
+ * to set the default parameter values for the first time.
+ */
+ qed_dbg_grc_init_params(p_hwfn);
+
+ if (grc_param >= MAX_DBG_GRC_PARAMS)
+ return DBG_STATUS_INVALID_ARGS;
+ if (val < s_grc_param_defs[grc_param].min ||
+ val > s_grc_param_defs[grc_param].max)
+ return DBG_STATUS_INVALID_ARGS;
+
+ if (s_grc_param_defs[grc_param].is_preset) {
+ /* Preset param */
+
+ /* Disabling a preset is not allowed. Call
+ * dbg_grc_set_params_default instead.
+ */
+ if (!val)
+ return DBG_STATUS_INVALID_ARGS;
+
+ /* Update all params with the preset values */
+ for (i = 0; i < MAX_DBG_GRC_PARAMS; i++) {
+ u32 preset_val;
+
+ /* Skip persistent params */
+ if (s_grc_param_defs[i].is_persistent)
+ continue;
+
+ /* Find preset value */
+ if (grc_param == DBG_GRC_PARAM_EXCLUDE_ALL)
+ preset_val =
+ s_grc_param_defs[i].exclude_all_preset_val;
+ else if (grc_param == DBG_GRC_PARAM_CRASH)
+ preset_val =
+ s_grc_param_defs[i].crash_preset_val;
+ else
+ return DBG_STATUS_INVALID_ARGS;
+
+ qed_grc_set_param(p_hwfn,
+ (enum dbg_grc_params)i, preset_val);
+ }
+ } else {
+ /* Regular param - set its value */
+ qed_grc_set_param(p_hwfn, grc_param, val);
+ }
+
+ return DBG_STATUS_OK;
+}
+
/* Assign default GRC param values */
void qed_dbg_grc_set_params_default(struct qed_hwfn *p_hwfn)
{
@@ -7997,9 +8069,16 @@ static u32 qed_calc_regdump_header(enum debug_print_features feature,
int qed_dbg_all_data(struct qed_dev *cdev, void *buffer)
{
u8 cur_engine, omit_engine = 0, org_engine;
+ struct qed_hwfn *p_hwfn =
+ &cdev->hwfns[cdev->dbg_params.engine_for_debug];
+ struct dbg_tools_data *dev_data = &p_hwfn->dbg_info;
+ int grc_params[MAX_DBG_GRC_PARAMS], i;
u32 offset = 0, feature_size;
int rc;
+ for (i = 0; i < MAX_DBG_GRC_PARAMS; i++)
+ grc_params[i] = dev_data->grc.param_val[i];
+
if (cdev->num_hwfns == 1)
omit_engine = 1;
@@ -8087,6 +8166,9 @@ int qed_dbg_all_data(struct qed_dev *cdev, void *buffer)
rc);
}
+ for (i = 0; i < MAX_DBG_GRC_PARAMS; i++)
+ dev_data->grc.param_val[i] = grc_params[i];
+
/* GRC dump - must be last because when mcp stuck it will
* clutter idle_chk, reg_fifo, ...
*/
diff --git a/drivers/net/ethernet/qlogic/qed/qed_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
index 557a12e..cf3ceb6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
@@ -3024,6 +3024,21 @@ void qed_read_regs(struct qed_hwfn *p_hwfn,
*/
bool qed_read_fw_info(struct qed_hwfn *p_hwfn,
struct qed_ptt *p_ptt, struct fw_info *fw_info);
+/**
+ * @brief qed_dbg_grc_config - Sets the value of a GRC parameter.
+ *
+ * @param p_hwfn - HW device data
+ * @param grc_param - GRC parameter
+ * @param val - Value to set.
+ *
+ * @return error if one of the following holds:
+ * - the version wasn't set
+ * - grc_param is invalid
+ * - val is outside the allowed boundaries
+ */
+enum dbg_status qed_dbg_grc_config(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ enum dbg_grc_params grc_param, u32 val);
/**
* @brief qed_dbg_grc_set_params_default - Reverts all GRC parameters to their
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index c9a7571..ac1511a8 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -2583,6 +2583,26 @@ static int qed_read_module_eeprom(struct qed_dev *cdev, char *buf,
return rc;
}
+static int qed_set_grc_config(struct qed_dev *cdev, u32 cfg_id, u32 val)
+{
+ struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
+ struct qed_ptt *ptt;
+ int rc = 0;
+
+ if (IS_VF(cdev))
+ return 0;
+
+ ptt = qed_ptt_acquire(hwfn);
+ if (!ptt)
+ return -EAGAIN;
+
+ rc = qed_dbg_grc_config(hwfn, ptt, cfg_id, val);
+
+ qed_ptt_release(hwfn, ptt);
+
+ return rc;
+}
+
static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
{
return QED_AFFIN_HWFN_IDX(cdev);
@@ -2637,6 +2657,7 @@ static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
.read_module_eeprom = &qed_read_module_eeprom,
.get_affin_hwfn_idx = &qed_get_affin_hwfn_idx,
.read_nvm_cfg = &qed_nvm_flash_cfg_read,
+ .set_grc_config = &qed_set_grc_config,
};
void qed_get_protocol_stats(struct qed_dev *cdev,
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index 06fd958..e354638 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -1143,6 +1143,15 @@ struct qed_common_ops {
*/
int (*read_nvm_cfg)(struct qed_dev *cdev, u8 **buf, u32 cmd,
u32 entity_id);
+
+/**
+ * @brief set_grc_config - Configure value for grc config id.
+ * @param cdev
+ * @param cfg_id - grc config id
+ * @param val - grc config value
+ *
+ */
+ int (*set_grc_config)(struct qed_dev *cdev, u32 cfg_id, u32 val);
};
#define MASK_FIELD(_name, _value) \
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 4/4] qede: Add support for dumping the grc data.
From: Sudarsana Reddy Kalluru @ 2019-08-30 7:42 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190830074206.8836-1-skalluru@marvell.com>
This patch adds driver support for configuring grc dump config flags, and
dumping the grc data via ethtool get/set-dump interfaces.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qede/qede.h | 1 +
drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 29 +++++++++++++++++++++++--
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index 8f2adde..c303a92 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -181,6 +181,7 @@ enum qede_flags_bit {
enum qede_dump_cmd {
QEDE_DUMP_CMD_NONE = 0,
QEDE_DUMP_CMD_NVM_CFG,
+ QEDE_DUMP_CMD_GRCDUMP,
QEDE_DUMP_CMD_MAX
};
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 2359293..ec27a43 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -2001,6 +2001,10 @@ static int qede_set_dump(struct net_device *dev, struct ethtool_dump *val)
edev->dump_info.args[edev->dump_info.num_args] = val->flag;
edev->dump_info.num_args++;
break;
+ case QEDE_DUMP_CMD_GRCDUMP:
+ rc = edev->ops->common->set_grc_config(edev->cdev,
+ val->flag, 1);
+ break;
default:
break;
}
@@ -2013,14 +2017,24 @@ static int qede_get_dump_flag(struct net_device *dev,
{
struct qede_dev *edev = netdev_priv(dev);
+ if (!edev->ops || !edev->ops->common) {
+ DP_ERR(edev, "Edev ops not populated\n");
+ return -EINVAL;
+ }
+
dump->version = QEDE_DUMP_VERSION;
switch (edev->dump_info.cmd) {
case QEDE_DUMP_CMD_NVM_CFG:
dump->flag = QEDE_DUMP_CMD_NVM_CFG;
dump->len = QEDE_DUMP_NVM_BUF_LEN;
break;
- default:
+ case QEDE_DUMP_CMD_GRCDUMP:
+ dump->flag = QEDE_DUMP_CMD_GRCDUMP;
+ dump->len = edev->ops->common->dbg_all_data_size(edev->cdev);
break;
+ default:
+ DP_ERR(edev, "Invalid cmd = %d\n", edev->dump_info.cmd);
+ return -EINVAL;
}
DP_VERBOSE(edev, QED_MSG_DEBUG,
@@ -2033,7 +2047,14 @@ static int qede_get_dump_data(struct net_device *dev,
struct ethtool_dump *dump, void *buf)
{
struct qede_dev *edev = netdev_priv(dev);
- int rc;
+ int rc = 0;
+
+ if (!edev->ops || !edev->ops->common) {
+ DP_ERR(edev, "Edev ops not populated\n");
+ edev->dump_info.cmd = QEDE_DUMP_CMD_NONE;
+ edev->dump_info.num_args = 0;
+ return -EINVAL;
+ }
switch (edev->dump_info.cmd) {
case QEDE_DUMP_CMD_NVM_CFG:
@@ -2047,6 +2068,10 @@ static int qede_get_dump_data(struct net_device *dev,
edev->dump_info.args[0],
edev->dump_info.args[1]);
break;
+ case QEDE_DUMP_CMD_GRCDUMP:
+ memset(buf, 0, dump->len);
+ rc = edev->ops->common->dbg_all_data(edev->cdev, buf);
+ break;
default:
DP_ERR(edev, "Invalid cmd = %d\n", edev->dump_info.cmd);
rc = -EINVAL;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 0/2] net: dsa: microchip: add KSZ8563 support
From: Razvan Stefanescu @ 2019-08-30 7:52 UTC (permalink / raw)
To: Woojung Huh, Microchip Linux Driver Support, Andrew Lunn,
Vivien Didelot, Florian Fainelli, David S . Miller
Cc: netdev, linux-kernel, Razvan Stefanescu
This patchset adds compatibility string for the KSZ8563 switch.
Razvan Stefanescu (2):
dt-bindings: net: dsa: document additional Microchip KSZ8563 switch
net: dsa: microchip: add KSZ8563 compatibility string
Documentation/devicetree/bindings/net/dsa/ksz.txt | 1 +
drivers/net/dsa/microchip/ksz9477_spi.c | 1 +
2 files changed, 2 insertions(+)
--
Changelog:
v2: drop fix patches
2.20.1
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: net: dsa: document additional Microchip KSZ8563 switch
From: Razvan Stefanescu @ 2019-08-30 7:52 UTC (permalink / raw)
To: Woojung Huh, Microchip Linux Driver Support, Andrew Lunn,
Vivien Didelot, Florian Fainelli, David S . Miller
Cc: netdev, linux-kernel, Razvan Stefanescu
In-Reply-To: <20190830075202.20740-1-razvan.stefanescu@microchip.com>
It is a 3-Port 10/100 Ethernet Switch with 1588v2 PTP.
Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
Changelog
v2: no update
Documentation/devicetree/bindings/net/dsa/ksz.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 5e8429b6f9ca..95e91e84151c 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -15,6 +15,7 @@ Required properties:
- "microchip,ksz8565"
- "microchip,ksz9893"
- "microchip,ksz9563"
+ - "microchip,ksz8563"
Optional properties:
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/2] net: dsa: microchip: add KSZ8563 compatibility string
From: Razvan Stefanescu @ 2019-08-30 7:52 UTC (permalink / raw)
To: Woojung Huh, Microchip Linux Driver Support, Andrew Lunn,
Vivien Didelot, Florian Fainelli, David S . Miller
Cc: netdev, linux-kernel, Razvan Stefanescu
In-Reply-To: <20190830075202.20740-1-razvan.stefanescu@microchip.com>
It is a 3-Port 10/100 Ethernet Switch with 1588v2 PTP.
Signed-off-by: Razvan Stefanescu <razvan.stefanescu@microchip.com>
---
Changelog
v2: no update
drivers/net/dsa/microchip/ksz9477_spi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index a226b389e12d..2e402e4d866f 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -80,6 +80,7 @@ static const struct of_device_id ksz9477_dt_ids[] = {
{ .compatible = "microchip,ksz9897" },
{ .compatible = "microchip,ksz9893" },
{ .compatible = "microchip,ksz9563" },
+ { .compatible = "microchip,ksz8563" },
{},
};
MODULE_DEVICE_TABLE(of, ksz9477_dt_ids);
--
2.20.1
^ permalink raw reply related
* Re: [RESEND PATCH 0/5] Add bluetooth support for Orange Pi 3
From: Marcel Holtmann @ 2019-08-30 7:53 UTC (permalink / raw)
To: megous
Cc: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Johan Hedberg,
Mark Rutland, David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth
In-Reply-To: <20190823103139.17687-1-megous@megous.com>
Hi Ondrej,
> (Resend to add missing lists, sorry for the noise.)
>
> This series implements bluetooth support for Xunlong Orange Pi 3 board.
>
> The board uses AP6256 WiFi/BT 5.0 chip.
>
> Summary of changes:
>
> - add more delay to let initialize the chip
> - let the kernel detect firmware file path
> - add new compatible and update dt-bindings
> - update Orange Pi 3 / H6 DTS
>
> Please take a look.
>
> thank you and regards,
> Ondrej Jirman
>
> Ondrej Jirman (5):
> dt-bindings: net: Add compatible for BCM4345C5 bluetooth device
> bluetooth: bcm: Add support for loading firmware for BCM4345C5
> bluetooth: hci_bcm: Give more time to come out of reset
> arm64: dts: allwinner: h6: Add pin configs for uart1
> arm64: dts: allwinner: orange-pi-3: Enable UART1 / Bluetooth
>
> .../bindings/net/broadcom-bluetooth.txt | 1 +
> .../dts/allwinner/sun50i-h6-orangepi-3.dts | 19 +++++++++++++++++++
> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
> drivers/bluetooth/btbcm.c | 3 +++
> drivers/bluetooth/hci_bcm.c | 3 ++-
> 5 files changed, 35 insertions(+), 1 deletion(-)
all 5 patches have been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox