* Re: [PATCH bpf-next v6 02/12] ixgbe: simplify Rx buffer recycle
From: Jonathan Lemon @ 2019-08-30 15:39 UTC (permalink / raw)
To: Kevin Laatz
Cc: netdev, ast, daniel, bjorn.topel, magnus.karlsson, jakub.kicinski,
saeedm, maximmi, stephen, bruce.richardson, ciara.loftus, bpf,
intel-wired-lan
In-Reply-To: <20190827022531.15060-3-kevin.laatz@intel.com>
On 26 Aug 2019, at 19:25, Kevin Laatz wrote:
> Currently, the dma, addr and handle are modified when we reuse Rx buffers
> in zero-copy mode. However, this is not required as the inputs to the
> function are copies, not the original values themselves. As we use the
> copies within the function, we can use the original 'obi' values
> directly without having to mask and add the headroom.
>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf-next v6 01/12] i40e: simplify Rx buffer recycle
From: Jonathan Lemon @ 2019-08-30 15:37 UTC (permalink / raw)
To: Kevin Laatz
Cc: netdev, ast, daniel, bjorn.topel, magnus.karlsson, jakub.kicinski,
saeedm, maximmi, stephen, bruce.richardson, ciara.loftus, bpf,
intel-wired-lan
In-Reply-To: <20190827022531.15060-2-kevin.laatz@intel.com>
On 26 Aug 2019, at 19:25, Kevin Laatz wrote:
> Currently, the dma, addr and handle are modified when we reuse Rx buffers
> in zero-copy mode. However, this is not required as the inputs to the
> function are copies, not the original values themselves. As we use the
> copies within the function, we can use the original 'old_bi' values
> directly without having to mask and add the headroom.
>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: flow_offload: mangle action at byte level
From: Vlad Buslov @ 2019-08-30 15:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netfilter-devel@vger.kernel.org, davem@davemloft.net,
netdev@vger.kernel.org, vishal@chelsio.com,
jakub.kicinski@netronome.com, Saeed Mahameed, jiri@resnulli.us
In-Reply-To: <20190830005336.23604-4-pablo@netfilter.org>
On Fri 30 Aug 2019 at 03:53, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> The flow mangle action is originally modeled after the tc pedit action,
> this has a number of shortcomings:
>
> 1) The tc pedit offset must be set on the 32-bits boundaries. Many
> protocol header field offsets are not aligned to 32-bits, eg. port
> destination, port source and ethernet destination. This patch adjusts
> the offset accordingly and trim off length in these case, so drivers get
> an exact offset and length to the header fields.
>
> 2) The maximum mangle length is one word of 32-bits, hence you need to
> up to four actions to mangle an IPv6 address. This patch coalesces
> consecutive tc pedit actions into one single action so drivers can
> configure the IPv6 mangling in one go. Ethernet address fields now
> require one single action instead of two too.
>
> The following drivers have been updated accordingly to use this new
> mangle action layout:
>
> 1) The cxgb4 driver does not need to split protocol field matching
> larger than one 32-bit words into multiple definitions. Instead one
> single definition per protocol field is enough. Checking for
> transport protocol ports is also simplified.
>
> 2) The mlx5 driver logic to disallow IPv4 ttl and IPv6 hoplimit fields
> becomes more simple too.
>
> 3) The nfp driver uses the nfp_fl_set_helper() function to configure the
> payload mangling. The memchr_inv() function is used to check for
> proper initialization of the value and mask. The driver has been
> updated to refer to the exact protocol header offsets too.
>
> As a result, this patch reduces code complexity on the driver side at
> the cost of adding ~100 LOC at the core to perform offset and length
> adjustment; and to coalesce consecutive actions.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
[...]
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index e30a151d8527..e8827fa8263a 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -3289,11 +3289,128 @@ void tc_cleanup_flow_action(struct flow_action *flow_action)
> }
> EXPORT_SYMBOL(tc_cleanup_flow_action);
>
> +static unsigned int flow_action_mangle_type(enum pedit_cmd cmd)
> +{
> + switch (cmd) {
> + case TCA_PEDIT_KEY_EX_CMD_SET:
> + return FLOW_ACTION_MANGLE;
> + case TCA_PEDIT_KEY_EX_CMD_ADD:
> + return FLOW_ACTION_ADD;
> + default:
> + WARN_ON_ONCE(1);
> + }
> + return 0;
> +}
> +
> +struct flow_action_mangle_ctx {
> + u8 cmd;
> + u8 offset;
> + u8 htype;
> + u8 idx;
> + u8 val[FLOW_ACTION_MANGLE_MAXLEN];
> + u8 mask[FLOW_ACTION_MANGLE_MAXLEN];
> + u32 first_word;
> + u32 last_word;
> +};
> +
> +static void flow_action_mangle_entry(struct flow_action_entry *entry,
> + const struct flow_action_mangle_ctx *ctx)
> +{
> + u32 delta;
> +
> + entry->id = ctx->cmd;
> + entry->mangle.htype = ctx->htype;
> + entry->mangle.offset = ctx->offset;
> + entry->mangle.len = ctx->idx;
> +
> + /* Adjust offset. */
> + delta = sizeof(u32) - (fls(ctx->first_word) / BITS_PER_BYTE);
> + entry->mangle.offset += delta;
> +
> + /* Adjust length. */
> + entry->mangle.len -= ((ffs(ctx->last_word) / BITS_PER_BYTE) + delta);
> +
> + /* Copy value and mask from offset using the adjusted length. */
> + memcpy(entry->mangle.val, &ctx->val[delta], entry->mangle.len);
> + memcpy(entry->mangle.mask, &ctx->mask[delta], entry->mangle.len);
> +}
> +
> +static void flow_action_mangle_ctx_update(struct flow_action_mangle_ctx *ctx,
> + const struct tc_action *act, int k)
> +{
> + u32 val, mask;
> +
> + val = tcf_pedit_val(act, k);
> + mask = ~tcf_pedit_mask(act, k);
> +
> + memcpy(&ctx->val[ctx->idx], &val, sizeof(u32));
> + memcpy(&ctx->mask[ctx->idx], &mask, sizeof(u32));
> + ctx->idx += sizeof(u32);
> +}
> +
> +static void flow_action_mangle_ctx_init(struct flow_action_mangle_ctx *ctx,
> + const struct tc_action *act, int k)
> +{
> + ctx->cmd = flow_action_mangle_type(tcf_pedit_cmd(act, k));
> + ctx->offset = tcf_pedit_offset(act, k);
> + ctx->htype = tcf_pedit_htype(act, k);
> + ctx->idx = 0;
> +
> + ctx->first_word = ntohl(~tcf_pedit_mask(act, k));
> + ctx->last_word = ctx->first_word;
> +
> + flow_action_mangle_ctx_update(ctx, act, k);
> +}
> +
> +static int flow_action_mangle(struct flow_action *flow_action,
> + struct flow_action_entry *entry,
> + const struct tc_action *act, int j)
> +{
> + struct flow_action_mangle_ctx ctx;
> + int k;
> +
> + if (tcf_pedit_cmd(act, 0) > TCA_PEDIT_KEY_EX_CMD_ADD)
> + return -1;
> +
> + flow_action_mangle_ctx_init(&ctx, act, 0);
> +
> + /* Special case: one single 32-bits word. */
> + if (tcf_pedit_nkeys(act) == 1) {
> + flow_action_mangle_entry(entry, &ctx);
> + return j;
> + }
> +
> + for (k = 1; k < tcf_pedit_nkeys(act); k++) {
> + if (tcf_pedit_cmd(act, k) > TCA_PEDIT_KEY_EX_CMD_ADD)
> + return -1;
> +
> + /* Offset is contiguous and type is the same, coalesce. */
> + if (ctx.idx < FLOW_ACTION_MANGLE_MAXLEN &&
> + ctx.offset + ctx.idx == tcf_pedit_offset(act, k) &&
> + ctx.cmd == flow_action_mangle_type(tcf_pedit_cmd(act, k))) {
> + flow_action_mangle_ctx_update(&ctx, act, k);
> + continue;
Hi Pablo,
With this change you coalesce multiple pedits into single
flow_action_entry, which means that resulting rule->action.num_entries
is incorrect because number of filled flow actions entries will be less
than num_actions. With this, I get mlx5 rejecting such flow_rule with
"mlx5_core: The offload action is not supported." due to trailing
unfilled flow action(s) with id==0.
> + }
> + ctx.last_word = ntohl(~tcf_pedit_mask(act, k - 1));
> +
> + /* Cannot coalesce, set up this entry. */
> + flow_action_mangle_entry(entry, &ctx);
> +
> + flow_action_mangle_ctx_init(&ctx, act, k);
> + entry = &flow_action->entries[++j];
> + }
> +
> + ctx.last_word = ntohl(~tcf_pedit_mask(act, k - 1));
> + flow_action_mangle_entry(entry, &ctx);
> +
> + return j;
> +}
> +
> int tc_setup_flow_action(struct flow_action *flow_action,
> const struct tcf_exts *exts, bool rtnl_held)
> {
> const struct tc_action *act;
> - int i, j, k, err = 0;
> + int i, j, err = 0;
>
> if (!exts)
> return 0;
> @@ -3366,25 +3483,9 @@ int tc_setup_flow_action(struct flow_action *flow_action,
> } else if (is_tcf_tunnel_release(act)) {
> entry->id = FLOW_ACTION_TUNNEL_DECAP;
> } else if (is_tcf_pedit(act)) {
> - for (k = 0; k < tcf_pedit_nkeys(act); k++) {
> - switch (tcf_pedit_cmd(act, k)) {
> - case TCA_PEDIT_KEY_EX_CMD_SET:
> - entry->id = FLOW_ACTION_MANGLE;
> - break;
> - case TCA_PEDIT_KEY_EX_CMD_ADD:
> - entry->id = FLOW_ACTION_ADD;
> - break;
> - default:
> - err = -EOPNOTSUPP;
> - goto err_out;
> - }
> - entry->mangle.htype = tcf_pedit_htype(act, k);
> - entry->mangle.mask = ~tcf_pedit_mask(act, k);
> - entry->mangle.val = tcf_pedit_val(act, k) &
> - entry->mangle.mask;
> - entry->mangle.offset = tcf_pedit_offset(act, k);
> - entry = &flow_action->entries[++j];
> - }
> + j = flow_action_mangle(flow_action, entry, act, j);
> + if (j < 0)
> + goto err_out;
> } else if (is_tcf_csum(act)) {
> entry->id = FLOW_ACTION_CSUM;
> entry->csum_flags = tcf_csum_update_flags(act);
> @@ -3439,8 +3540,7 @@ int tc_setup_flow_action(struct flow_action *flow_action,
> goto err_out;
> }
>
> - if (!is_tcf_pedit(act))
> - j++;
> + j++;
> }
>
> err_out:
^ permalink raw reply
* Re: [PATCH 2/3] samples: pktgen: add helper functions for IP(v4/v6) CIDR parsing
From: Daniel T. Lee @ 2019-08-30 15:27 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: David S . Miller, netdev
In-Reply-To: <20190830152758.41b38c24@carbon>
On Fri, Aug 30, 2019 at 10:28 PM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Thu, 29 Aug 2019 05:42:42 +0900
> "Daniel T. Lee" <danieltimlee@gmail.com> wrote:
>
> > This commit adds CIDR parsing and IP validate helper function to parse
> > single IP or range of IP with CIDR. (e.g. 198.18.0.0/15)
> >
> > Helpers will be used in prior to set target address in samples/pktgen.
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> > ---
> > samples/pktgen/functions.sh | 134 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 134 insertions(+)
> >
> > diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
> > index 4af4046d71be..eb1c52e25018 100644
> > --- a/samples/pktgen/functions.sh
> > +++ b/samples/pktgen/functions.sh
> > @@ -163,6 +163,140 @@ function get_node_cpus()
> > echo $node_cpu_list
> > }
> >
> > +# Extend shrunken IPv6 address.
> > +# fe80::42:bcff:fe84:e10a => fe80:0:0:0:42:bcff:fe84:e10a
> > +function extend_addr6()
> > +{
> > + local addr=$1
> > + local sep=:
> > + local sep2=::
> > + local sep_cnt=$(tr -cd $sep <<< $1 | wc -c)
> > + local shrink
> > +
> > + # separator count : should be between 2, 7.
> > + if [[ $sep_cnt -lt 2 || $sep_cnt -gt 7 ]]; then
> > + err 5 "Invalid IP6 address sep: $1"
> > + fi
> > +
> > + # if shrink '::' occurs multiple, it's malformed.
> > + shrink=( $(egrep -o "$sep{2,}" <<< $addr) )
> > + if [[ ${#shrink[@]} -ne 0 ]]; then
> > + if [[ ${#shrink[@]} -gt 1 || ( ${shrink[0]} != $sep2 ) ]]; then
> > + err 5 "Invalid IP$IP6 address shr: $1"
> > + fi
> > + fi
> > +
> > + # add 0 at begin & end, and extend addr by adding :0
> > + [[ ${addr:0:1} == $sep ]] && addr=0${addr}
> > + [[ ${addr: -1} == $sep ]] && addr=${addr}0
> > + echo "${addr/$sep2/$(printf ':0%.s' $(seq $[8-sep_cnt])):}"
> > +}
> > +
> > +
> > +# Given a single IP(v4/v6) address, whether it is valid.
> > +function validate_addr()
> > +{
> > + # check function is called with (funcname)6
> > + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6
> > + local len=$[ IP6 ? 8 : 4 ]
> > + local max=$[ 2**(len*2)-1 ]
> > + local addr
> > + local sep
> > +
> > + # set separator for each IP(v4/v6)
> > + [[ $IP6 ]] && sep=: || sep=.
> > + IFS=$sep read -a addr <<< $1
> > +
> > + # array length
> > + if [[ ${#addr[@]} != $len ]]; then
> > + err 5 "Invalid IP$IP6 address: $1"
> > + fi
> > +
> > + # check each digit between 0, $max
> > + for digit in "${addr[@]}"; do
> > + [[ $IP6 ]] && digit=$[ 16#$digit ]
> > + if [[ $digit -lt 0 || $digit -gt $max ]]; then
> > + err 5 "Invalid IP$IP6 address: $1"
> > + fi
> > + done
> > +
> > + return 0
> > +}
> > +
> > +function validate_addr6() { validate_addr $@ ; }
> > +
> > +# Given a single IP(v4/v6) or CIDR, return minimum and maximum IP addr.
> > +function parse_addr()
>
> I must say that I'm impressed by your bash-shell skills, BUT below
> function does look too complicated for doing this... I were expecting
> that you would use the regular & (AND) operation to do the prefix
> masking.
>
>
Thank you very much for your compliment!
I'll switch to a more intuitive code as soon as possible.
(It might take some time to send next version of patch,
but i'll try to send it asap.)
And also, thank you for taking your time for the review.
> > +{
> > + # check function is called with (funcname)6
> > + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6
> > + local bitlen=$[ IP6 ? 128 : 32 ]
> > +
> > + local addr=$1
> > + local net
> > + local prefix
> > + local min_ip
> > + local max_ip
> > +
> > + IFS='/' read net prefix <<< $addr
> > + [[ $IP6 ]] && net=$(extend_addr6 $net)
> > + validate_addr$IP6 $net
> > +
> > + if [[ $prefix -gt $bitlen ]]; then
> > + err 5 "Invalid prefix: $prefix"
> > + elif [[ -z $prefix ]]; then
> > + min_ip=$net
> > + max_ip=$net
> > + else
> > + # defining array for converting Decimal 2 Binary
> > + # 00000000 00000001 00000010 00000011 00000100 ...
> > + local d2b='{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}'
> > + [[ $IP6 ]] && d2b+=$d2b
> > + eval local D2B=($d2b)
> > +
> > + local shift=$[ bitlen-prefix ]
> > + local ip_bit
> > + local ip
> > + local sep
> > +
> > + # set separator for each IP(v4/v6)
> > + [[ $IP6 ]] && sep=: || sep=.
> > + IFS=$sep read -ra ip <<< $net
> > +
> > + # build full size bit
> > + for digit in "${ip[@]}"; do
> > + [[ $IP6 ]] && digit=$[ 16#$digit ]
> > + ip_bit+=${D2B[$digit]}
> > + done
> > +
> > + # fill 0 or 1 by $shift
> > + base_bit=${ip_bit::$prefix}
> > + min_bit="$base_bit$(printf '0%.s' $(seq $shift))"
> > + max_bit="$base_bit$(printf '1%.s' $(seq $shift))"
> > +
> > + bit2addr() {
> > + local step=$[ IP6 ? 16 : 8 ]
> > + local max=$[ bitlen-step ]
> > + local result
> > + local fmt
> > + [[ $IP6 ]] && fmt='%X' || fmt='%d'
> > +
> > + for i in $(seq 0 $step $max); do
> > + result+=$(printf $fmt $[ 2#${1:$i:$step} ])
> > + [[ $i != $max ]] && result+=$sep
> > + done
> > + echo $result
> > + }
> > +
> > + min_ip=$(bit2addr $min_bit)
> > + max_ip=$(bit2addr $max_bit)
> > + fi
> > +
> > + echo $min_ip $max_ip
> > +}
> > +
> > +function parse_addr6() { parse_addr $@ ; }
> > +
> > # Given a single or range of port(s), return minimum and maximum port number.
> > function parse_ports()
> > {
>
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Qian Cai @ 2019-08-30 15:25 UTC (permalink / raw)
To: Eric Dumazet, davem; +Cc: netdev, linux-mm, linux-kernel
In-Reply-To: <6109dab4-4061-8fee-96ac-320adf94e130@gmail.com>
On Fri, 2019-08-30 at 17:11 +0200, Eric Dumazet wrote:
>
> On 8/30/19 4:57 PM, Qian Cai wrote:
> > When running heavy memory pressure workloads, the system is throwing
> > endless warnings below due to the allocation could fail from
> > __build_skb(), and the volume of this call could be huge which may
> > generate a lot of serial console output and cosumes all CPUs as
> > warn_alloc() could be expensive by calling dump_stack() and then
> > show_mem().
> >
> > Fix it by silencing the warning in this call site. Also, it seems
> > unnecessary to even print a warning at all if the allocation failed in
> > __build_skb(), as it may just retransmit the packet and retry.
> >
>
> Same patches are showing up there and there from time to time.
>
> Why is this particular spot interesting, against all others not adding
> __GFP_NOWARN ?
>
> Are we going to have hundred of patches adding __GFP_NOWARN at various points,
> or should we get something generic to not flood the syslog in case of memory
> pressure ?
>
From my testing which uses LTP oom* tests. There are only 3 places need to be
patched. The other two are in IOMMU code for both Intel and AMD. The place is
particular interesting because it could cause the system with floating serial
console output for days without making progress in OOM. I suppose it ends up in
a looping condition that warn_alloc() would end up generating more calls into
__build_skb() via ksoftirqd.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/3] bpf: implement CAP_BPF
From: Nicolas Dichtel @ 2019-08-30 15:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Alexei Starovoitov, luto, davem, peterz, rostedt, netdev, bpf,
kernel-team, linux-api
In-Reply-To: <20190829173034.up5g74onaekp53zd@ast-mbp.dhcp.thefacebook.com>
Le 29/08/2019 à 19:30, Alexei Starovoitov a écrit :
[snip]
> These are the links that showing that k8 can delegates caps.
> Are you saying that you know of folks who specifically
> delegate cap_sys_admin and cap_net_admin _only_ to a container to run bpf in there?
>
Yes, we need cap_sys_admin only to load bpf:
tc filter add dev eth0 ingress matchall action bpf obj ./tc_test_kern.o sec test
I'm not sure to understand why cap_net_admin is not enough to run the previous
command (ie why load is forbidden).
I want to avoid sys_admin, thus cap_bpf will be ok. But we need to manage the
backward compatibility.
Regards,
Nicolas
^ permalink raw reply
* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Eric Dumazet @ 2019-08-30 15:11 UTC (permalink / raw)
To: Qian Cai, davem; +Cc: netdev, linux-mm, linux-kernel
In-Reply-To: <1567177025-11016-1-git-send-email-cai@lca.pw>
On 8/30/19 4:57 PM, Qian Cai wrote:
> When running heavy memory pressure workloads, the system is throwing
> endless warnings below due to the allocation could fail from
> __build_skb(), and the volume of this call could be huge which may
> generate a lot of serial console output and cosumes all CPUs as
> warn_alloc() could be expensive by calling dump_stack() and then
> show_mem().
>
> Fix it by silencing the warning in this call site. Also, it seems
> unnecessary to even print a warning at all if the allocation failed in
> __build_skb(), as it may just retransmit the packet and retry.
>
Same patches are showing up there and there from time to time.
Why is this particular spot interesting, against all others not adding __GFP_NOWARN ?
Are we going to have hundred of patches adding __GFP_NOWARN at various points,
or should we get something generic to not flood the syslog in case of memory pressure ?
^ permalink raw reply
* Re: [PATCH] seccomp: fix compilation errors in seccomp-bpf kselftest
From: shuah @ 2019-08-30 14:59 UTC (permalink / raw)
To: Alakesh Haloi, Kees Cook, Andy Lutomirski, Will Drewry,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song
Cc: linux-kselftest, netdev, bpf, linux-kernel, shuah
In-Reply-To: <30e993fe-de76-9831-7ecc-61fcbcd51ae0@kernel.org>
On 8/30/19 8:09 AM, shuah wrote:
> On 8/22/19 3:58 PM, Alakesh Haloi wrote:
>> Without this patch we see following error while building and kselftest
>> for secccomp_bpf fails.
>>
>> seccomp_bpf.c:1787:20: error: ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’
>> undeclared (first use in this function);
>> seccomp_bpf.c:1788:6: error: ‘PTRACE_EVENTMSG_SYSCALL_EXIT’ undeclared
>> (first use in this function);
>>
>> Signed-off-by: Alakesh Haloi <alakesh.haloi@gmail.com>
>> ---
>> tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c
>> b/tools/testing/selftests/seccomp/seccomp_bpf.c
>> index 6ef7f16c4cf5..2e619760fc3e 100644
>> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
>> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
>> @@ -1353,6 +1353,14 @@ TEST_F(precedence, log_is_fifth_in_any_order)
>> #define PTRACE_EVENT_SECCOMP 7
>> #endif
>> +#ifndef PTRACE_EVENTMSG_SYSCALL_ENTRY
>> +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 1
>> +#endif
>> +
>> +#ifndef PTRACE_EVENTMSG_SYSCALL_EXIT
>> +#define PTRACE_EVENTMSG_SYSCALL_EXIT 2
>> +#endif
>> +
>> #define IS_SECCOMP_EVENT(status) ((status >> 16) ==
>> PTRACE_EVENT_SECCOMP)
>> bool tracer_running;
>> void tracer_stop(int sig)
>>
>
> Hi Kees,
>
> Okay to apply this one for 5.4-rc1. Or is this going through bpf tree?
> If it is going through bpf tree:
>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>
> thanks,
> -- Shuah
>
I saw your mail about Tycho's solution to be your preferred. Ignore this
message. I am applying Tycho's patch.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH] net/skbuff: silence warnings under memory pressure
From: Qian Cai @ 2019-08-30 14:57 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-mm, linux-kernel, Qian Cai
When running heavy memory pressure workloads, the system is throwing
endless warnings below due to the allocation could fail from
__build_skb(), and the volume of this call could be huge which may
generate a lot of serial console output and cosumes all CPUs as
warn_alloc() could be expensive by calling dump_stack() and then
show_mem().
Fix it by silencing the warning in this call site. Also, it seems
unnecessary to even print a warning at all if the allocation failed in
__build_skb(), as it may just retransmit the packet and retry.
NMI watchdog: Watchdog detected hard LOCKUP on cpu 7
Hardware name: HP ProLiant XL420 Gen9/ProLiant XL420 Gen9, BIOS U19
12/27/2015
RIP: 0010:dump_stack+0xd/0x9a
Code: 5d c3 48 c7 c2 c0 ce aa bb 4c 89 ee 48 c7 c7 60 32 e3 ae e8 b3 3e
81 ff e9 ab ff ff ff 55 48 89 e5 41 55 41 83 cd ff 41 54 53 <9c> 41 5c
fa be 04 00 00 00 48 c7 c7 80 42 63 af 65 8b 1d f6 7c 8c
RSP: 0018:ffff888452389670 EFLAGS: 00000086
RAX: 000000000000000b RBX: 0000000000000007 RCX: ffffffffae75143f
RDX: 0000000000000001 RSI: 0000000000000004 RDI: ffffffffaf634280
RBP: ffff888452389688 R08: 0000000000000004 R09: fffffbfff5ec6850
R10: fffffbfff5ec6850 R11: 0000000000000003 R12: 0000000000000086
R13: 00000000ffffffff R14: ffff88820547c040 R15: ffffffffaecc86a0
FS: 0000000000000000(0000) GS:ffff888452380000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f94f0537000 CR3: 00000005c8012006 CR4: 00000000001606a0
Call Trace:
<IRQ>
warn_alloc.cold.43+0x8a/0x148
__alloc_pages_nodemask+0x1a5c/0x1bb0
alloc_pages_current+0x9c/0x110
allocate_slab+0x34a/0x11f0
new_slab+0x46/0x70
___slab_alloc+0x604/0x950
__slab_alloc+0x12/0x20
kmem_cache_alloc+0x32a/0x400
__build_skb+0x23/0x60
build_skb+0x1a/0xb0
igb_clean_rx_irq+0xafc/0x1010 [igb]
igb_poll+0x4bb/0xe30 [igb]
net_rx_action+0x244/0x7a0
__do_softirq+0x1a0/0x60a
irq_exit+0xb5/0xd0
do_IRQ+0x81/0x170
common_interrupt+0xf/0xf
</IRQ>
RIP: 0010:cpuidle_enter_state+0x151/0x8d0
Code: 64 af e8 62 22 c2 ff 8b 05 04 f6 0b 01 85 c0 0f 8f 18 04 00 00 31
ff e8 9d 9e 97 ff 80 7d d0 00 0f 85 06 02 00 00 fb 45 85 ed <0f> 88 2d
02 00 00 4d 63 fd 49 83 ff 09 0f 87 8c 06 00 00 4b 8d 04
RSP: 0018:ffff888205487cf8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffffdc
RAX: 0000000000000000 RBX: ffffe8fc09596f98 RCX: ffffffffadf093da
RDX: dffffc0000000000 RSI: dffffc0000000000 RDI: ffff8884523c2128
RBP: ffff888205487d48 R08: fffffbfff5ec9d62 R09: fffffbfff5ec9d62
R10: fffffbfff5ec9d61 R11: ffffffffaf64eb0b R12: ffffffffaf459580
R13: 0000000000000004 R14: 0000101fb3d64a9b R15: ffffe8fc09596f9c
cpuidle_enter+0x41/0x70
call_cpuidle+0x5e/0x90
do_idle+0x313/0x340
cpu_startup_entry+0x1d/0x1f
start_secondary+0x28b/0x330
secondary_startup_64+0xb6/0xc0
Signed-off-by: Qian Cai <cai@lca.pw>
---
net/core/skbuff.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0338820ee0ec..9cc4148deddd 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -307,7 +307,9 @@ struct sk_buff *__build_skb(void *data, unsigned int frag_size)
{
struct sk_buff *skb;
- skb = kmem_cache_alloc(skbuff_head_cache, GFP_ATOMIC);
+ skb = kmem_cache_alloc(skbuff_head_cache,
+ GFP_ATOMIC | __GFP_NOWARN);
+
if (unlikely(!skb))
return NULL;
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Michal Kubecek @ 2019-08-30 14:49 UTC (permalink / raw)
To: netdev
Cc: Roopa Prabhu, Jiri Pirko, David Miller, Jakub Kicinski,
David Ahern, Stephen Hemminger, dcbw, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
In-Reply-To: <CAJieiUgGY4amm_z1VGgBF-3WZceah+R5OVLEi=O2RS8RGpC9dg@mail.gmail.com>
On Fri, Aug 30, 2019 at 07:35:23AM -0700, Roopa Prabhu wrote:
> On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
> > >
> > > yes, correct. I mentioned that because I was wondering if we can
> > > think along the same lines for this API.
> > > eg
> > > (a) RTM_NEWLINK always replaces the list attribute
> > > (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
> > > (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
> > >
> > > (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
> > > case. I have not looked at the full dellink path if it will work
> > > neatly..its been a busy day )
> >
> > AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
> > current code would ignore any future attribute in RTM_DELLINK message
> > (any kernel before the strict validation was introduced definitely will)
> > and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
> > unless I missed something, such message would result in deleting the
> > network device (if possible) with any kernel not implementing the
> > feature.
>
> ok, ack. yes today it does. I was hinting if that can be changed to
> support list update with a flag like the RTM_DELLINK AF_BRIDGE does
> for vlan list del.
What I wanted to say is that it would have to be done in a way that
would make current kernel (or even older, e.g. one with no strict
attribute checking at all) reject or at least ignore such request,
not delete the device.
Michal
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: David Ahern @ 2019-08-30 14:47 UTC (permalink / raw)
To: Roopa Prabhu, Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Andrew Lunn, parav, Saeed Mahameed,
mlxsw
In-Reply-To: <CAJieiUgGY4amm_z1VGgBF-3WZceah+R5OVLEi=O2RS8RGpC9dg@mail.gmail.com>
On 8/30/19 8:35 AM, Roopa Prabhu wrote:
> On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>>
>> On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
>>>
>>> yes, correct. I mentioned that because I was wondering if we can
>>> think along the same lines for this API.
>>> eg
>>> (a) RTM_NEWLINK always replaces the list attribute
>>> (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
>>> (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
>>>
>>> (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
>>> case. I have not looked at the full dellink path if it will work
>>> neatly..its been a busy day )
>>
>> AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
>> current code would ignore any future attribute in RTM_DELLINK message
>> (any kernel before the strict validation was introduced definitely will)
>> and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
>> unless I missed something, such message would result in deleting the
>> network device (if possible) with any kernel not implementing the
>> feature.
>
> ok, ack. yes today it does. I was hinting if that can be changed to
> support list update with a flag like the RTM_DELLINK AF_BRIDGE does
> for vlan list del.
>
> so to summarize, i think we have discussed the following options to
> update a netlink list attribute so far:
> (a) encode an optional attribute/flag in the list attribute in
> RTM_SETLINK to indicate if it is a add or del
The ALT_IFNAME attribute could also be a struct that has both the string
and a flag.
> (b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
> (close to bridge vlan add/del)
> (c) introduce a separate generic msg type to add/del to a list
> attribute (IIUC this does need a separate msg type per subsystem or
> netlink API)
>
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Roopa Prabhu @ 2019-08-30 14:35 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Jakub Kicinski, David Ahern,
Stephen Hemminger, dcbw, Andrew Lunn, parav, Saeed Mahameed,
mlxsw
In-Reply-To: <20190829052620.GK29594@unicorn.suse.cz>
On Wed, Aug 28, 2019 at 10:26 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Wed, Aug 28, 2019 at 09:36:41PM -0700, Roopa Prabhu wrote:
> >
> > yes, correct. I mentioned that because I was wondering if we can
> > think along the same lines for this API.
> > eg
> > (a) RTM_NEWLINK always replaces the list attribute
> > (b) RTM_SETLINK with NLM_F_APPEND always appends to the list attribute
> > (c) RTM_DELLINK with NLM_F_APPEND updates the list attribute
> >
> > (It could be NLM_F_UPDATE if NLM_F_APPEND sounds weird in the del
> > case. I have not looked at the full dellink path if it will work
> > neatly..its been a busy day )
>
> AFAICS rtnl_dellink() calls nlmsg_parse_deprecated() so that even
> current code would ignore any future attribute in RTM_DELLINK message
> (any kernel before the strict validation was introduced definitely will)
> and it does not seem to check NLM_F_APPEND or NLM_F_UPDATE either. So
> unless I missed something, such message would result in deleting the
> network device (if possible) with any kernel not implementing the
> feature.
ok, ack. yes today it does. I was hinting if that can be changed to
support list update with a flag like the RTM_DELLINK AF_BRIDGE does
for vlan list del.
so to summarize, i think we have discussed the following options to
update a netlink list attribute so far:
(a) encode an optional attribute/flag in the list attribute in
RTM_SETLINK to indicate if it is a add or del
(b) Use a flag in RTM_SETLINK and RTM_DELINK to indicate add/del
(close to bridge vlan add/del)
(c) introduce a separate generic msg type to add/del to a list
attribute (IIUC this does need a separate msg type per subsystem or
netlink API)
^ permalink raw reply
* Re: bug report: libceph: follow redirect replies from osds
From: Ilya Dryomov @ 2019-08-30 14:26 UTC (permalink / raw)
To: Colin Ian King
Cc: Jeff Layton, Sage Weil, David S. Miller, Ceph Development,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <3a4ff829-7302-7201-81c2-a557fe35afc8@canonical.com>
On Fri, Aug 30, 2019 at 4:05 PM Colin Ian King <colin.king@canonical.com> wrote:
>
> Hi,
>
> Static analysis with Coverity has picked up an issue with commit:
>
> commit 205ee1187a671c3b067d7f1e974903b44036f270
> Author: Ilya Dryomov <ilya.dryomov@inktank.com>
> Date: Mon Jan 27 17:40:20 2014 +0200
>
> libceph: follow redirect replies from osds
>
> Specifically in function ceph_redirect_decode in net/ceph/osd_client.c:
>
> 3485
> 3486 len = ceph_decode_32(p);
>
> CID 17904: Unused value (UNUSED_VALUE)
>
> 3487 *p += len; /* skip osd_instructions */
> 3488
> 3489 /* skip the rest */
> 3490 *p = struct_end;
>
> The double write to *p looks wrong, I suspect the *p += len; should be
> just incrementing pointer p as in: p += len. Am I correct to assume
> this is the correct fix?
Hi Colin,
No, the double write to *p is correct. It skips over len bytes and
then skips to the end of the redirect reply. There is no bug here but
we could drop
len = ceph_decode_32(p);
*p += len; /* skip osd_instructions */
and skip to the end directly to make Coverity happier.
Thanks,
Ilya
^ permalink raw reply
* Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
From: Leonardo Bras @ 2019-08-30 14:15 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, David S. Miller, netfilter-devel, coreteam,
netdev, linux-kernel, Jozsef Kadlecsik, Alexey Kuznetsov,
Hideaki YOSHIFUJI
In-Reply-To: <20190829205832.GM20113@breakpoint.cc>
[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]
On Thu, 2019-08-29 at 22:58 +0200, Florian Westphal wrote:
> In any case your patch looks ok to me.
Great! Please give your feedback on v3:
http://patchwork.ozlabs.org/patch/1154040/
[...]
>
> Even if we disable call-ip6tables in br_netfilter we will at least
> in addition need a patch for nft_fib_netdev.c.
>
> From a "avoid calls to ipv6 stack when its disabled" standpoint,
> the safest fix is to disable call-ip6tables functionality if ipv6
> module is off *and* fix nft_fib_netdev.c to BREAK in ipv6 is off case.
>
> I started to place a list of suspicous modules here, but that got out
> of hand quickly.
>
> So, given I don't want to plaster ipv6_mod_enabled() everywhere, I
> would suggest this course of action:
>
> 1. add a patch to BREAK in nft_fib_netdev.c for !ipv6_mod_enabled()
> 2. change net/bridge/br_netfilter_hooks.c, br_nf_pre_routing() to
> make sure ipv6_mod_enabled() is true before doing the ipv6 stack
> "emulation".
>
> Makes sense?
IMHO sure.
Shortly, I will send a couple patches proposing the above changes.
(Or my best understanding about them :) )
>
> Thanks,
> Florian
Thank you,
Leonardo Bras
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] seccomp: fix compilation errors in seccomp-bpf kselftest
From: shuah @ 2019-08-30 14:09 UTC (permalink / raw)
To: Alakesh Haloi, Kees Cook, Andy Lutomirski, Will Drewry,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song
Cc: linux-kselftest, netdev, bpf, linux-kernel, shuah
In-Reply-To: <20190822215823.GA11292@ip-172-31-44-144.us-west-2.compute.internal>
On 8/22/19 3:58 PM, Alakesh Haloi wrote:
> Without this patch we see following error while building and kselftest
> for secccomp_bpf fails.
>
> seccomp_bpf.c:1787:20: error: ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’ undeclared (first use in this function);
> seccomp_bpf.c:1788:6: error: ‘PTRACE_EVENTMSG_SYSCALL_EXIT’ undeclared (first use in this function);
>
> Signed-off-by: Alakesh Haloi <alakesh.haloi@gmail.com>
> ---
> tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index 6ef7f16c4cf5..2e619760fc3e 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -1353,6 +1353,14 @@ TEST_F(precedence, log_is_fifth_in_any_order)
> #define PTRACE_EVENT_SECCOMP 7
> #endif
>
> +#ifndef PTRACE_EVENTMSG_SYSCALL_ENTRY
> +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 1
> +#endif
> +
> +#ifndef PTRACE_EVENTMSG_SYSCALL_EXIT
> +#define PTRACE_EVENTMSG_SYSCALL_EXIT 2
> +#endif
> +
> #define IS_SECCOMP_EVENT(status) ((status >> 16) == PTRACE_EVENT_SECCOMP)
> bool tracer_running;
> void tracer_stop(int sig)
>
Hi Kees,
Okay to apply this one for 5.4-rc1. Or is this going through bpf tree?
If it is going through bpf tree:
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
thanks,
-- Shuah
^ permalink raw reply
* bug report: libceph: follow redirect replies from osds
From: Colin Ian King @ 2019-08-30 14:05 UTC (permalink / raw)
To: Ilya Dryomov, Jeff Layton, Sage Weil, David S. Miller, ceph-devel,
netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Hi,
Static analysis with Coverity has picked up an issue with commit:
commit 205ee1187a671c3b067d7f1e974903b44036f270
Author: Ilya Dryomov <ilya.dryomov@inktank.com>
Date: Mon Jan 27 17:40:20 2014 +0200
libceph: follow redirect replies from osds
Specifically in function ceph_redirect_decode in net/ceph/osd_client.c:
3485
3486 len = ceph_decode_32(p);
CID 17904: Unused value (UNUSED_VALUE)
3487 *p += len; /* skip osd_instructions */
3488
3489 /* skip the rest */
3490 *p = struct_end;
The double write to *p looks wrong, I suspect the *p += len; should be
just incrementing pointer p as in: p += len. Am I correct to assume
this is the correct fix?
Colin
^ permalink raw reply
* Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
From: Cornelia Huck @ 2019-08-30 14:02 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB486621283F935B673455DA63D1BD0@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Fri, 30 Aug 2019 12:58:04 +0000
Parav Pandit <parav@mellanox.com> wrote:
> > -----Original Message-----
> > From: Cornelia Huck <cohuck@redhat.com>
> > Sent: Friday, August 30, 2019 6:09 PM
> > To: Parav Pandit <parav@mellanox.com>
> > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
> >
> > On Fri, 30 Aug 2019 12:33:22 +0000
> > Parav Pandit <parav@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Cornelia Huck <cohuck@redhat.com>
> > > > Sent: Friday, August 30, 2019 2:47 PM
> > > > To: Parav Pandit <parav@mellanox.com>
> > > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > > Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
> > > >
> > > > On Thu, 29 Aug 2019 06:18:59 -0500
> > > > Parav Pandit <parav@mellanox.com> wrote:
> > > >
> > > > > Some vendor drivers want an identifier for an mdev device that is
> > > > > shorter than the UUID, due to length restrictions in the consumers
> > > > > of that identifier.
> > > > >
> > > > > Add a callback that allows a vendor driver to request an alias of
> > > > > a specified length to be generated for an mdev device. If
> > > > > generated, that alias is checked for collisions.
> > > > >
> > > > > It is an optional attribute.
> > > > > mdev alias is generated using sha1 from the mdev name.
> > > > >
> > > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > > >
> > > > > ---
> > > > > Changelog:
> > > > > v1->v2:
> > > > > - Kept mdev_device naturally aligned
> > > > > - Added error checking for crypt_*() calls
> > > > > - Corrected a typo from 'and' to 'an'
> > > > > - Changed return type of generate_alias() from int to char*
> > > > > v0->v1:
> > > > > - Moved alias length check outside of the parent lock
> > > > > - Moved alias and digest allocation from kvzalloc to kzalloc
> > > > > - &alias[0] changed to alias
> > > > > - alias_length check is nested under get_alias_length callback
> > > > > check
> > > > > - Changed comments to start with an empty line
> > > > > - Fixed cleaunup of hash if mdev_bus_register() fails
> > > > > - Added comment where alias memory ownership is handed over to
> > > > > mdev device
> > > > > - Updated commit log to indicate motivation for this feature
> > > > > ---
> > > > > drivers/vfio/mdev/mdev_core.c | 123
> > > > ++++++++++++++++++++++++++++++-
> > > > > drivers/vfio/mdev/mdev_private.h | 5 +-
> > > > > drivers/vfio/mdev/mdev_sysfs.c | 13 ++--
> > > > > include/linux/mdev.h | 4 +
> > > > > 4 files changed, 135 insertions(+), 10 deletions(-)
> >
> > > > ...and detached from the local variable here. Who is freeing it? The
> > > > comment states that it is done by the mdev, but I don't see it?
> > > >
> > > mdev_device_free() frees it.
> >
> > Ah yes, I overlooked the kfree().
> >
> > > once its assigned to mdev, mdev is the owner of it.
> > >
> > > > This detour via the local variable looks weird to me. Can you either
> > > > create the alias directly in the mdev (would need to happen later in
> > > > the function, but I'm not sure why you generate the alias before
> > > > checking for duplicates anyway), or do an explicit copy?
> > > Alias duplicate check is done after generating it, because duplicate alias are
> > not allowed.
> > > The probability of collision is rare.
> > > So it is speculatively generated without hold the lock, because there is no
> > need to hold the lock.
> > > It is compared along with guid while mutex lock is held in single loop.
> > > And if it is duplicate, there is no need to allocate mdev.
> > >
> > > It will be sub optimal to run through the mdev list 2nd time after mdev
> > creation and after generating alias for duplicate check.
> >
> > Ok, but what about copying it? I find this "set local variable to NULL after
> > ownership is transferred" pattern a bit unintuitive. Copying it to the mdev (and
> > then unconditionally freeing it) looks more obvious to me.
> Its not unconditionally freed.
That's not what I have been saying :(
> Its freed in the error unwinding path.
> I think its ok along with the comment that describes this error path area.
It is not wrong, but I'm not sure I like it.
^ permalink raw reply
* Re: [PATCH 2/3] samples: pktgen: add helper functions for IP(v4/v6) CIDR parsing
From: Jesper Dangaard Brouer @ 2019-08-30 13:27 UTC (permalink / raw)
To: Daniel T. Lee; +Cc: David S . Miller, netdev, brouer
In-Reply-To: <20190828204243.16666-2-danieltimlee@gmail.com>
On Thu, 29 Aug 2019 05:42:42 +0900
"Daniel T. Lee" <danieltimlee@gmail.com> wrote:
> This commit adds CIDR parsing and IP validate helper function to parse
> single IP or range of IP with CIDR. (e.g. 198.18.0.0/15)
>
> Helpers will be used in prior to set target address in samples/pktgen.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> ---
> samples/pktgen/functions.sh | 134 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 134 insertions(+)
>
> diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
> index 4af4046d71be..eb1c52e25018 100644
> --- a/samples/pktgen/functions.sh
> +++ b/samples/pktgen/functions.sh
> @@ -163,6 +163,140 @@ function get_node_cpus()
> echo $node_cpu_list
> }
>
> +# Extend shrunken IPv6 address.
> +# fe80::42:bcff:fe84:e10a => fe80:0:0:0:42:bcff:fe84:e10a
> +function extend_addr6()
> +{
> + local addr=$1
> + local sep=:
> + local sep2=::
> + local sep_cnt=$(tr -cd $sep <<< $1 | wc -c)
> + local shrink
> +
> + # separator count : should be between 2, 7.
> + if [[ $sep_cnt -lt 2 || $sep_cnt -gt 7 ]]; then
> + err 5 "Invalid IP6 address sep: $1"
> + fi
> +
> + # if shrink '::' occurs multiple, it's malformed.
> + shrink=( $(egrep -o "$sep{2,}" <<< $addr) )
> + if [[ ${#shrink[@]} -ne 0 ]]; then
> + if [[ ${#shrink[@]} -gt 1 || ( ${shrink[0]} != $sep2 ) ]]; then
> + err 5 "Invalid IP$IP6 address shr: $1"
> + fi
> + fi
> +
> + # add 0 at begin & end, and extend addr by adding :0
> + [[ ${addr:0:1} == $sep ]] && addr=0${addr}
> + [[ ${addr: -1} == $sep ]] && addr=${addr}0
> + echo "${addr/$sep2/$(printf ':0%.s' $(seq $[8-sep_cnt])):}"
> +}
> +
> +
> +# Given a single IP(v4/v6) address, whether it is valid.
> +function validate_addr()
> +{
> + # check function is called with (funcname)6
> + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6
> + local len=$[ IP6 ? 8 : 4 ]
> + local max=$[ 2**(len*2)-1 ]
> + local addr
> + local sep
> +
> + # set separator for each IP(v4/v6)
> + [[ $IP6 ]] && sep=: || sep=.
> + IFS=$sep read -a addr <<< $1
> +
> + # array length
> + if [[ ${#addr[@]} != $len ]]; then
> + err 5 "Invalid IP$IP6 address: $1"
> + fi
> +
> + # check each digit between 0, $max
> + for digit in "${addr[@]}"; do
> + [[ $IP6 ]] && digit=$[ 16#$digit ]
> + if [[ $digit -lt 0 || $digit -gt $max ]]; then
> + err 5 "Invalid IP$IP6 address: $1"
> + fi
> + done
> +
> + return 0
> +}
> +
> +function validate_addr6() { validate_addr $@ ; }
> +
> +# Given a single IP(v4/v6) or CIDR, return minimum and maximum IP addr.
> +function parse_addr()
I must say that I'm impressed by your bash-shell skills, BUT below
function does look too complicated for doing this... I were expecting
that you would use the regular & (AND) operation to do the prefix
masking.
> +{
> + # check function is called with (funcname)6
> + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6
> + local bitlen=$[ IP6 ? 128 : 32 ]
> +
> + local addr=$1
> + local net
> + local prefix
> + local min_ip
> + local max_ip
> +
> + IFS='/' read net prefix <<< $addr
> + [[ $IP6 ]] && net=$(extend_addr6 $net)
> + validate_addr$IP6 $net
> +
> + if [[ $prefix -gt $bitlen ]]; then
> + err 5 "Invalid prefix: $prefix"
> + elif [[ -z $prefix ]]; then
> + min_ip=$net
> + max_ip=$net
> + else
> + # defining array for converting Decimal 2 Binary
> + # 00000000 00000001 00000010 00000011 00000100 ...
> + local d2b='{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}'
> + [[ $IP6 ]] && d2b+=$d2b
> + eval local D2B=($d2b)
> +
> + local shift=$[ bitlen-prefix ]
> + local ip_bit
> + local ip
> + local sep
> +
> + # set separator for each IP(v4/v6)
> + [[ $IP6 ]] && sep=: || sep=.
> + IFS=$sep read -ra ip <<< $net
> +
> + # build full size bit
> + for digit in "${ip[@]}"; do
> + [[ $IP6 ]] && digit=$[ 16#$digit ]
> + ip_bit+=${D2B[$digit]}
> + done
> +
> + # fill 0 or 1 by $shift
> + base_bit=${ip_bit::$prefix}
> + min_bit="$base_bit$(printf '0%.s' $(seq $shift))"
> + max_bit="$base_bit$(printf '1%.s' $(seq $shift))"
> +
> + bit2addr() {
> + local step=$[ IP6 ? 16 : 8 ]
> + local max=$[ bitlen-step ]
> + local result
> + local fmt
> + [[ $IP6 ]] && fmt='%X' || fmt='%d'
> +
> + for i in $(seq 0 $step $max); do
> + result+=$(printf $fmt $[ 2#${1:$i:$step} ])
> + [[ $i != $max ]] && result+=$sep
> + done
> + echo $result
> + }
> +
> + min_ip=$(bit2addr $min_bit)
> + max_ip=$(bit2addr $max_bit)
> + fi
> +
> + echo $min_ip $max_ip
> +}
> +
> +function parse_addr6() { parse_addr $@ ; }
> +
> # Given a single or range of port(s), return minimum and maximum port number.
> function parse_ports()
> {
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [RESEND PATCH 0/5] Add bluetooth support for Orange Pi 3
From: Maxime Ripard @ 2019-08-30 13:20 UTC (permalink / raw)
To: Marcel Holtmann
Cc: megous, Chen-Yu Tsai, Rob Herring, Johan Hedberg, Mark Rutland,
David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth
In-Reply-To: <D02B89FB-F8C0-40AD-A99A-6C1B4FEB72A0@holtmann.org>
On Fri, Aug 30, 2019 at 02:43:48PM +0200, Marcel Holtmann wrote:
> >>> (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.
> >
> > The DTS patches (last 2) should go through the arm-soc tree, can you
> > drop them?
>
> why is that? We have included DTS changes for Bluetooth devices
> directly all the time. What is different with this hardware?
I guess some maintainers are more relaxed with it than we are then,
but for the why, well, it's the usual reasons, the most immediate one
being that it reduces to a minimum the conflicts between trees.
The other being that it's not really usual to merge patches supposed
to be handled by another maintainer without (at least) his
consent. I'm pretty sure you would have asked the same request if I
would have merged the bluetooth patches through my tree without
notice.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* RE: [PATCH v2 5/6] mdev: Update sysfs documentation
From: Parav Pandit @ 2019-08-30 13:10 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830144927.7961193e.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 6:19 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 5/6] mdev: Update sysfs documentation
>
> On Thu, 29 Aug 2019 06:19:03 -0500
> Parav Pandit <parav@mellanox.com> wrote:
>
> > Updated documentation for optional read only sysfs attribute.
>
> I'd probably merge this into the patch introducing the attribute.
>
Ok. I will spin v3.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > ---
> > Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/driver-api/vfio-mediated-device.rst
> > b/Documentation/driver-api/vfio-mediated-device.rst
> > index 25eb7d5b834b..0ab03d3f5629 100644
> > --- a/Documentation/driver-api/vfio-mediated-device.rst
> > +++ b/Documentation/driver-api/vfio-mediated-device.rst
> > @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev
> Device
> > |--- remove
> > |--- mdev_type {link to its type}
> > |--- vendor-specific-attributes [optional]
> > + |--- alias [optional]
>
> "optional" implies "not always present" to me, not "might return a read error if
> not available". Don't know if there's a better way to tag this? Or make it really
> optional? :)
May be write it as,
alias [ optional when requested by parent ]
>
> >
> > * remove (write only)
> >
> > @@ -281,6 +282,10 @@ Example::
> >
> > # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
> >
> > +* alias (read only)
> > +Whenever a parent requested to generate an alias, each mdev is
> > +assigned a unique alias by the mdev core. This file shows the alias of the
> mdev device.
>
> It's not really the parent, but the vendor driver requesting this, right? Also,
At mdev level, it only knows parent->ops structure, whether parent is registered by vendor driver or something else.
> "each mdev" is a bit ambiguous,
It is in context of the parent. Sentence is not starting with "each mdev".
But may be more verbosely written as,
Whenever a parent requested to generate an alias, Each mdev device of such parent is assigned
unique alias by the mdev core. This file shows the alias of the mdev device.
> created via that driver. Lastly, if we stick with the "returns an error if not
> implemented" approach, that should also be mentioned here.
Ok. Will spin v3 to describe it.
>
> > +
> > Mediated device Hot plug
> > ------------------------
> >
^ permalink raw reply
* RE: [PATCH v2 2/6] mdev: Make mdev alias unique among all mdevs
From: Parav Pandit @ 2019-08-30 12:59 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830144052.11d23ec3.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 6:11 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 2/6] mdev: Make mdev alias unique among all mdevs
>
> On Thu, 29 Aug 2019 06:19:00 -0500
> Parav Pandit <parav@mellanox.com> wrote:
>
> > Mdev alias should be unique among all the mdevs, so that when such
> > alias is used by the mdev users to derive other objects, there is no
> > collision in a given system.
> >
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> >
> > ---
> > Changelog:
> > v1->v2:
> > - Moved alias NULL check at beginning
> > v0->v1:
> > - Fixed inclusiong of alias for NULL check
> > - Added ratelimited debug print for sha1 hash collision error
> > ---
> > drivers/vfio/mdev/mdev_core.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/vfio/mdev/mdev_core.c
> > b/drivers/vfio/mdev/mdev_core.c index 3bdff0469607..c9bf2ac362b9
> > 100644
> > --- a/drivers/vfio/mdev/mdev_core.c
> > +++ b/drivers/vfio/mdev/mdev_core.c
> > @@ -388,6 +388,13 @@ int mdev_device_create(struct kobject *kobj, struct
> device *dev,
> > ret = -EEXIST;
> > goto mdev_fail;
> > }
> > + if (alias && tmp->alias && strcmp(alias, tmp->alias) == 0) {
> > + mutex_unlock(&mdev_list_lock);
> > + ret = -EEXIST;
> > + dev_dbg_ratelimited(dev, "Hash collision in alias
> creation for UUID %pUl\n",
> > + uuid);
> > + goto mdev_fail;
> > + }
> > }
> >
> > mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
>
> Any reason not to merge this into the first patch?
No. It surely can be merged. Its easy to start with smaller patches instead of splitting. :-)
Doing uniqueness comparison was easy to split as independent functionality, so did as 2nd patch.
But either way is ok.
^ permalink raw reply
* RE: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
From: Parav Pandit @ 2019-08-30 12:58 UTC (permalink / raw)
To: Cornelia Huck
Cc: alex.williamson@redhat.com, Jiri Pirko, kwankhede@nvidia.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190830143927.163d13a7.cohuck@redhat.com>
> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Friday, August 30, 2019 6:09 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
>
> On Fri, 30 Aug 2019 12:33:22 +0000
> Parav Pandit <parav@mellanox.com> wrote:
>
> > > -----Original Message-----
> > > From: Cornelia Huck <cohuck@redhat.com>
> > > Sent: Friday, August 30, 2019 2:47 PM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > kwankhede@nvidia.com; davem@davemloft.net; kvm@vger.kernel.org;
> > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH v2 1/6] mdev: Introduce sha1 based mdev alias
> > >
> > > On Thu, 29 Aug 2019 06:18:59 -0500
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >
> > > > Some vendor drivers want an identifier for an mdev device that is
> > > > shorter than the UUID, due to length restrictions in the consumers
> > > > of that identifier.
> > > >
> > > > Add a callback that allows a vendor driver to request an alias of
> > > > a specified length to be generated for an mdev device. If
> > > > generated, that alias is checked for collisions.
> > > >
> > > > It is an optional attribute.
> > > > mdev alias is generated using sha1 from the mdev name.
> > > >
> > > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > >
> > > > ---
> > > > Changelog:
> > > > v1->v2:
> > > > - Kept mdev_device naturally aligned
> > > > - Added error checking for crypt_*() calls
> > > > - Corrected a typo from 'and' to 'an'
> > > > - Changed return type of generate_alias() from int to char*
> > > > v0->v1:
> > > > - Moved alias length check outside of the parent lock
> > > > - Moved alias and digest allocation from kvzalloc to kzalloc
> > > > - &alias[0] changed to alias
> > > > - alias_length check is nested under get_alias_length callback
> > > > check
> > > > - Changed comments to start with an empty line
> > > > - Fixed cleaunup of hash if mdev_bus_register() fails
> > > > - Added comment where alias memory ownership is handed over to
> > > > mdev device
> > > > - Updated commit log to indicate motivation for this feature
> > > > ---
> > > > drivers/vfio/mdev/mdev_core.c | 123
> > > ++++++++++++++++++++++++++++++-
> > > > drivers/vfio/mdev/mdev_private.h | 5 +-
> > > > drivers/vfio/mdev/mdev_sysfs.c | 13 ++--
> > > > include/linux/mdev.h | 4 +
> > > > 4 files changed, 135 insertions(+), 10 deletions(-)
>
> > > ...and detached from the local variable here. Who is freeing it? The
> > > comment states that it is done by the mdev, but I don't see it?
> > >
> > mdev_device_free() frees it.
>
> Ah yes, I overlooked the kfree().
>
> > once its assigned to mdev, mdev is the owner of it.
> >
> > > This detour via the local variable looks weird to me. Can you either
> > > create the alias directly in the mdev (would need to happen later in
> > > the function, but I'm not sure why you generate the alias before
> > > checking for duplicates anyway), or do an explicit copy?
> > Alias duplicate check is done after generating it, because duplicate alias are
> not allowed.
> > The probability of collision is rare.
> > So it is speculatively generated without hold the lock, because there is no
> need to hold the lock.
> > It is compared along with guid while mutex lock is held in single loop.
> > And if it is duplicate, there is no need to allocate mdev.
> >
> > It will be sub optimal to run through the mdev list 2nd time after mdev
> creation and after generating alias for duplicate check.
>
> Ok, but what about copying it? I find this "set local variable to NULL after
> ownership is transferred" pattern a bit unintuitive. Copying it to the mdev (and
> then unconditionally freeing it) looks more obvious to me.
Its not unconditionally freed. Its freed in the error unwinding path.
I think its ok along with the comment that describes this error path area.
^ permalink raw reply
* Re: [PATCH 1/3] samples: pktgen: make variable consistent with option
From: Jesper Dangaard Brouer @ 2019-08-30 12:57 UTC (permalink / raw)
To: Daniel T. Lee; +Cc: David S . Miller, netdev, brouer
In-Reply-To: <20190828204243.16666-1-danieltimlee@gmail.com>
On Thu, 29 Aug 2019 05:42:41 +0900
"Daniel T. Lee" <danieltimlee@gmail.com> wrote:
> This commit changes variable names that can cause confusion.
>
> For example, variable DST_MIN is quite confusing since the
> keyword 'udp_dst_min' and keyword 'dst_min' is used with pg_ctrl.
>
> On the following commit, 'dst_min' will be used to set destination IP,
> and the existing variable name DST_MIN should be changed.
>
> Variable names are matched to the exact keyword used with pg_ctrl.
>
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v2 5/6] mdev: Update sysfs documentation
From: Cornelia Huck @ 2019-08-30 12:49 UTC (permalink / raw)
To: Parav Pandit
Cc: alex.williamson, jiri, kwankhede, davem, kvm, linux-kernel,
netdev
In-Reply-To: <20190829111904.16042-6-parav@mellanox.com>
On Thu, 29 Aug 2019 06:19:03 -0500
Parav Pandit <parav@mellanox.com> wrote:
> Updated documentation for optional read only sysfs attribute.
I'd probably merge this into the patch introducing the attribute.
>
> Signed-off-by: Parav Pandit <parav@mellanox.com>
> ---
> Documentation/driver-api/vfio-mediated-device.rst | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst
> index 25eb7d5b834b..0ab03d3f5629 100644
> --- a/Documentation/driver-api/vfio-mediated-device.rst
> +++ b/Documentation/driver-api/vfio-mediated-device.rst
> @@ -270,6 +270,7 @@ Directories and Files Under the sysfs for Each mdev Device
> |--- remove
> |--- mdev_type {link to its type}
> |--- vendor-specific-attributes [optional]
> + |--- alias [optional]
"optional" implies "not always present" to me, not "might return a read
error if not available". Don't know if there's a better way to tag
this? Or make it really optional? :)
>
> * remove (write only)
>
> @@ -281,6 +282,10 @@ Example::
>
> # echo 1 > /sys/bus/mdev/devices/$mdev_UUID/remove
>
> +* alias (read only)
> +Whenever a parent requested to generate an alias, each mdev is assigned a unique
> +alias by the mdev core. This file shows the alias of the mdev device.
It's not really the parent, but the vendor driver requesting this,
right? Also, "each mdev" is a bit ambiguous, as this is only true for
the subset of mdevs created via that driver. Lastly, if we stick with
the "returns an error if not implemented" approach, that should also be
mentioned here.
> +
> Mediated device Hot plug
> ------------------------
>
^ permalink raw reply
* Re: [RESEND PATCH 0/5] Add bluetooth support for Orange Pi 3
From: Marcel Holtmann @ 2019-08-30 12:43 UTC (permalink / raw)
To: Maxime Ripard
Cc: megous, Chen-Yu Tsai, Rob Herring, Johan Hedberg, Mark Rutland,
David S. Miller, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-bluetooth
In-Reply-To: <20190830092104.odipmbflounqpffo@flea>
Hi Maxime,
>>> (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.
>
> The DTS patches (last 2) should go through the arm-soc tree, can you
> drop them?
why is that? We have included DTS changes for Bluetooth devices directly all the time. What is different with this hardware?
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