Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 2/3] sctp: add sock_reuseport for the sock in __sctp_hash_endpoint
From: Xin Long @ 2018-11-12  9:58 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
In-Reply-To: <20181022141547.GL6634@localhost.localdomain>

On Mon, Oct 22, 2018 at 11:15 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Sun, Oct 21, 2018 at 12:43:37PM +0800, Xin Long wrote:
> > This is a part of sk_reuseport support for sctp. It defines a helper
> > sctp_bind_addrs_check() to check if the bind_addrs in two socks are
> > matched. It will add sock_reuseport if they are completely matched,
> > and return err if they are partly matched, and alloc sock_reuseport
> > if all socks are not matched at all.
> >
> > It will work until sk_reuseport support is added in
> > sctp_get_port_local() in the next patch.
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  include/net/sctp/sctp.h    |  2 +-
> >  include/net/sctp/structs.h |  2 ++
> >  net/core/sock_reuseport.c  |  1 +
> >  net/sctp/bind_addr.c       | 28 ++++++++++++++++++++++
> >  net/sctp/input.c           | 60 +++++++++++++++++++++++++++++++++++++++-------
> >  net/sctp/socket.c          |  3 +--
> >  6 files changed, 85 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> > index 8c2caa3..b8cd58d 100644
> > --- a/include/net/sctp/sctp.h
> > +++ b/include/net/sctp/sctp.h
> > @@ -152,7 +152,7 @@ int sctp_primitive_RECONF(struct net *net, struct sctp_association *asoc,
> >   */
> >  int sctp_rcv(struct sk_buff *skb);
> >  void sctp_v4_err(struct sk_buff *skb, u32 info);
> > -void sctp_hash_endpoint(struct sctp_endpoint *);
> > +int sctp_hash_endpoint(struct sctp_endpoint *ep);
> >  void sctp_unhash_endpoint(struct sctp_endpoint *);
> >  struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *,
> >                            struct sctphdr *, struct sctp_association **,
> > diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> > index a11f937..15d017f 100644
> > --- a/include/net/sctp/structs.h
> > +++ b/include/net/sctp/structs.h
> > @@ -1190,6 +1190,8 @@ int sctp_bind_addr_conflict(struct sctp_bind_addr *, const union sctp_addr *,
> >                        struct sctp_sock *, struct sctp_sock *);
> >  int sctp_bind_addr_state(const struct sctp_bind_addr *bp,
> >                        const union sctp_addr *addr);
> > +int sctp_bind_addrs_check(struct sctp_sock *sp,
> > +                       struct sctp_sock *sp2, int cnt2);
> >  union sctp_addr *sctp_find_unmatch_addr(struct sctp_bind_addr        *bp,
> >                                       const union sctp_addr   *addrs,
> >                                       int                     addrcnt,
> > diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
> > index ba5cba5..d8fe3e5 100644
> > --- a/net/core/sock_reuseport.c
> > +++ b/net/core/sock_reuseport.c
> > @@ -187,6 +187,7 @@ int reuseport_add_sock(struct sock *sk, struct sock *sk2, bool bind_inany)
> >               call_rcu(&old_reuse->rcu, reuseport_free_rcu);
> >       return 0;
> >  }
> > +EXPORT_SYMBOL(reuseport_add_sock);
> >
> >  void reuseport_detach_sock(struct sock *sk)
> >  {
> > diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
> > index 7df3704..78d0d93 100644
> > --- a/net/sctp/bind_addr.c
> > +++ b/net/sctp/bind_addr.c
> > @@ -337,6 +337,34 @@ int sctp_bind_addr_match(struct sctp_bind_addr *bp,
> >       return match;
> >  }
> >
> > +int sctp_bind_addrs_check(struct sctp_sock *sp,
> > +                       struct sctp_sock *sp2, int cnt2)
> > +{
> > +     struct sctp_bind_addr *bp2 = &sp2->ep->base.bind_addr;
> > +     struct sctp_bind_addr *bp = &sp->ep->base.bind_addr;
> > +     struct sctp_sockaddr_entry *laddr, *laddr2;
> > +     bool exist = false;
> > +     int cnt = 0;
> > +
> > +     rcu_read_lock();
> > +     list_for_each_entry_rcu(laddr, &bp->address_list, list) {
> > +             list_for_each_entry_rcu(laddr2, &bp2->address_list, list) {
> > +                     if (sp->pf->af->cmp_addr(&laddr->a, &laddr2->a) &&
> > +                         laddr->valid == laddr2->valid) {
>
> I think by here in the normal run laddr2->valid will always be true,
> but as is it gives the impression that it accepts 0 == 0 too, which
> would be bad.  May be on a fast BINDX_REM/BINDX_ADD it could trigger
> laddr2->valid = 0 in there, not sure.
>
> Anyway, may be '... laddr->valid && laddr2->valid' instead or you
> really want to allow the 0 == 0 case?
>
will improve it in v2. thanks.

> > +                             exist = true;
> > +                             goto next;
> > +                     }
> > +             }
> > +             cnt = 0;
> > +             break;
> > +next:
> > +             cnt++;
> > +     }
> > +     rcu_read_unlock();
> > +
> > +     return (cnt == cnt2) ? 0 : (exist ? -EEXIST : 1);
> > +}
> > +
> >  /* Does the address 'addr' conflict with any addresses in
> >   * the bp.
> >   */
> > diff --git a/net/sctp/input.c b/net/sctp/input.c
> > index 60ede89..6bfeb10 100644
> > --- a/net/sctp/input.c
> > +++ b/net/sctp/input.c
> > @@ -723,43 +723,87 @@ static int sctp_rcv_ootb(struct sk_buff *skb)
> >  }
> >
> >  /* Insert endpoint into the hash table.  */
> > -static void __sctp_hash_endpoint(struct sctp_endpoint *ep)
> > +static int __sctp_hash_endpoint(struct sctp_endpoint *ep)
> >  {
> > -     struct net *net = sock_net(ep->base.sk);
> > -     struct sctp_ep_common *epb;
> > +     struct sock *sk = ep->base.sk;
> > +     struct net *net = sock_net(sk);
> >       struct sctp_hashbucket *head;
> > +     struct sctp_ep_common *epb;
> >
> >       epb = &ep->base;
> > -
> >       epb->hashent = sctp_ep_hashfn(net, epb->bind_addr.port);
> >       head = &sctp_ep_hashtable[epb->hashent];
> >
> > +     if (sk->sk_reuseport) {
> > +             bool any = sctp_is_ep_boundall(sk);
> > +             struct sctp_ep_common *epb2;
> > +             struct list_head *list;
> > +             int cnt = 0, err = 1;
> > +
> > +             list_for_each(list, &ep->base.bind_addr.address_list)
> > +                     cnt++;
> > +
> > +             sctp_for_each_hentry(epb2, &head->chain) {
> > +                     struct sock *sk2 = epb2->sk;
> > +
> > +                     if (!net_eq(sock_net(sk2), net) || sk2 == sk ||
> > +                         !uid_eq(sock_i_uid(sk2), sock_i_uid(sk)) ||
> > +                         !sk2->sk_reuseport)
> > +                             continue;
> > +
> > +                     err = sctp_bind_addrs_check(sctp_sk(sk2),
> > +                                                 sctp_sk(sk), cnt);
> > +                     if (!err) {
> > +                             err = reuseport_add_sock(sk, sk2, any);
> > +                             if (err)
> > +                                     return err;
> > +                             break;
> > +                     } else if (err < 0) {
> > +                             return err;
> > +                     }
> > +             }
> > +
> > +             if (err) {
> > +                     err = reuseport_alloc(sk, any);
> > +                     if (err)
> > +                             return err;
> > +             }
> > +     }
> > +
> >       write_lock(&head->lock);
> >       hlist_add_head(&epb->node, &head->chain);
> >       write_unlock(&head->lock);
> > +     return 0;
> >  }
> >
> >  /* Add an endpoint to the hash. Local BH-safe. */
> > -void sctp_hash_endpoint(struct sctp_endpoint *ep)
> > +int sctp_hash_endpoint(struct sctp_endpoint *ep)
> >  {
> > +     int err;
> > +
> >       local_bh_disable();
> > -     __sctp_hash_endpoint(ep);
> > +     err = __sctp_hash_endpoint(ep);
> >       local_bh_enable();
> > +
> > +     return err;
> >  }
> >
> >  /* Remove endpoint from the hash table.  */
> >  static void __sctp_unhash_endpoint(struct sctp_endpoint *ep)
> >  {
> > -     struct net *net = sock_net(ep->base.sk);
> > +     struct sock *sk = ep->base.sk;
> >       struct sctp_hashbucket *head;
> >       struct sctp_ep_common *epb;
> >
> >       epb = &ep->base;
> >
> > -     epb->hashent = sctp_ep_hashfn(net, epb->bind_addr.port);
> > +     epb->hashent = sctp_ep_hashfn(sock_net(sk), epb->bind_addr.port);
> >
> >       head = &sctp_ep_hashtable[epb->hashent];
> >
> > +     if (rcu_access_pointer(sk->sk_reuseport_cb))
> > +             reuseport_detach_sock(sk);
> > +
> >       write_lock(&head->lock);
> >       hlist_del_init(&epb->node);
> >       write_unlock(&head->lock);
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index fc0386e..44e7d8c 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -7850,8 +7850,7 @@ static int sctp_listen_start(struct sock *sk, int backlog)
> >       }
> >
> >       sk->sk_max_ack_backlog = backlog;
> > -     sctp_hash_endpoint(ep);
> > -     return 0;
> > +     return sctp_hash_endpoint(ep);
> >  }
> >
> >  /*
> > --
> > 2.1.0
> >

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: support raw tracepoints in modules
From: Martin Lau @ 2018-11-12 20:05 UTC (permalink / raw)
  To: Matt Mullins
  Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
	Kernel Team, Jessica Yu, Steven Rostedt, Ingo Molnar,
	linux-kernel@vger.kernel.org
In-Reply-To: <20181109220632.3944136-1-mmullins@fb.com>

On Fri, Nov 09, 2018 at 02:06:32PM -0800, Matt Mullins wrote:
> Distributions build drivers as modules, including network and filesystem
> drivers which export numerous tracepoints.  This enables
> bpf(BPF_RAW_TRACEPOINT_OPEN) to attach to those tracepoints.
> 
> Signed-off-by: Matt Mullins <mmullins@fb.com>
> ---
> My apologies if this got duplicated to some of the recipients; I think I have
> SMTP working now.
> 
>  include/linux/module.h       |  4 ++
>  include/linux/trace_events.h |  8 ++-
>  kernel/bpf/syscall.c         | 11 +++--
>  kernel/module.c              |  5 ++
>  kernel/trace/bpf_trace.c     | 96 +++++++++++++++++++++++++++++++++++-
>  5 files changed, 117 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index fce6b4335e36..5f147dd5e709 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -432,6 +432,10 @@ struct module {
>  	unsigned int num_tracepoints;
>  	tracepoint_ptr_t *tracepoints_ptrs;
>  #endif
> +#ifdef CONFIG_BPF_EVENTS
> +	unsigned int num_bpf_raw_events;
> +	struct bpf_raw_event_map *bpf_raw_events;
> +#endif
>  #ifdef HAVE_JUMP_LABEL
>  	struct jump_entry *jump_entries;
>  	unsigned int num_jump_entries;
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index 4130a5497d40..8a62731673f7 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -471,7 +471,8 @@ void perf_event_detach_bpf_prog(struct perf_event *event);
>  int perf_event_query_prog_array(struct perf_event *event, void __user *info);
>  int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
>  int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
> -struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name);
> +struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name);
> +void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp);
>  int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
>  			    u32 *fd_type, const char **buf,
>  			    u64 *probe_offset, u64 *probe_addr);
> @@ -502,10 +503,13 @@ static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf
>  {
>  	return -EOPNOTSUPP;
>  }
> -static inline struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
> +static inline struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
>  {
>  	return NULL;
>  }
> +static inline void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
> +{
> +}
>  static inline int bpf_get_perf_event_info(const struct perf_event *event,
>  					  u32 *prog_id, u32 *fd_type,
>  					  const char **buf, u64 *probe_offset,
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index cf5040fd5434..c447aa1ce770 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1597,6 +1597,7 @@ static int bpf_raw_tracepoint_release(struct inode *inode, struct file *filp)
>  		bpf_probe_unregister(raw_tp->btp, raw_tp->prog);
>  		bpf_prog_put(raw_tp->prog);
>  	}
> +	bpf_put_raw_tracepoint(raw_tp->btp);
>  	kfree(raw_tp);
>  	return 0;
>  }
> @@ -1622,13 +1623,15 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
>  		return -EFAULT;
>  	tp_name[sizeof(tp_name) - 1] = 0;
>  
> -	btp = bpf_find_raw_tracepoint(tp_name);
> +	btp = bpf_get_raw_tracepoint(tp_name);
>  	if (!btp)
>  		return -ENOENT;
>  
>  	raw_tp = kzalloc(sizeof(*raw_tp), GFP_USER);
> -	if (!raw_tp)
> -		return -ENOMEM;
> +	if (!raw_tp) {
> +		err = -ENOMEM;
> +		goto out_put_btp;
> +	}
>  	raw_tp->btp = btp;
>  
>  	prog = bpf_prog_get_type(attr->raw_tracepoint.prog_fd,
> @@ -1656,6 +1659,8 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
>  	bpf_prog_put(prog);
>  out_free_tp:
>  	kfree(raw_tp);
> +out_put_btp:
> +	bpf_put_raw_tracepoint(btp);
>  	return err;
>  }
>  
> diff --git a/kernel/module.c b/kernel/module.c
> index 49a405891587..06ec68f08387 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3093,6 +3093,11 @@ static int find_module_sections(struct module *mod, struct load_info *info)
>  					     sizeof(*mod->tracepoints_ptrs),
>  					     &mod->num_tracepoints);
>  #endif
> +#ifdef CONFIG_BPF_EVENTS
> +	mod->bpf_raw_events = section_objs(info, "__bpf_raw_tp_map",
> +					   sizeof(*mod->bpf_raw_events),
> +					   &mod->num_bpf_raw_events);
> +#endif
>  #ifdef HAVE_JUMP_LABEL
>  	mod->jump_entries = section_objs(info, "__jump_table",
>  					sizeof(*mod->jump_entries),
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 08fcfe440c63..a12d28c123eb 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -17,6 +17,43 @@
>  #include "trace_probe.h"
>  #include "trace.h"
>  
> +#ifdef CONFIG_MODULES
> +struct bpf_trace_module {
> +	struct module *module;
> +	struct list_head list;
> +};
> +
> +static LIST_HEAD(bpf_trace_modules);
> +static DEFINE_MUTEX(bpf_module_mutex);
> +
> +static struct bpf_raw_event_map *bpf_get_raw_tracepoint_module(const char *name)
> +{
> +	struct bpf_raw_event_map *btp, *ret = NULL;
> +	struct bpf_trace_module *btm;
> +	unsigned int i;
> +
Nit. May be check for list_empty() first before taking mutex?

> +	mutex_lock(&bpf_module_mutex);
> +	list_for_each_entry(btm, &bpf_trace_modules, list) {
> +		for (i = 0; i < btm->module->num_bpf_raw_events; ++i) {
> +			btp = &btm->module->bpf_raw_events[i];
> +			if (!strcmp(btp->tp->name, name)) {
> +				if (try_module_get(btm->module))
> +					ret = btp;
> +				goto out;
> +			}
> +		}
> +	}
> +out:
> +	mutex_unlock(&bpf_module_mutex);
> +	return ret;
> +}
> +#else
> +static struct bpf_raw_event_map *bpf_get_raw_tracepoint_module(const char *name)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_MODULES */
> +
>  u64 bpf_get_stackid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>  u64 bpf_get_stack(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>  
> @@ -1074,7 +1111,7 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
>  extern struct bpf_raw_event_map __start__bpf_raw_tp[];
>  extern struct bpf_raw_event_map __stop__bpf_raw_tp[];
>  
> -struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
> +struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
>  {
>  	struct bpf_raw_event_map *btp = __start__bpf_raw_tp;
>  
> @@ -1082,7 +1119,16 @@ struct bpf_raw_event_map *bpf_find_raw_tracepoint(const char *name)
>  		if (!strcmp(btp->tp->name, name))
>  			return btp;
>  	}
> -	return NULL;
> +
> +	return bpf_get_raw_tracepoint_module(name);
> +}
> +
> +void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
> +{
> +	struct module *mod = __module_address((unsigned long)btp);
> +
> +	if (mod)
> +		module_put(mod);
>  }
>  
>  static __always_inline
> @@ -1220,3 +1266,49 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
>  
>  	return err;
>  }
> +
> +#ifdef CONFIG_MODULES
> +int bpf_event_notify(struct notifier_block *nb, unsigned long op, void *module)
> +{
> +	struct bpf_trace_module *btm, *tmp;
> +	struct module *mod = module;
> +
> +	if (mod->num_bpf_raw_events == 0)
Also check for MODULE_STATE_GOING and MODULE_STATE_COMING
before taking the mutex.

> +		return 0;
> +
> +	mutex_lock(&bpf_module_mutex);
> +
> +	switch (op) {
> +	case MODULE_STATE_COMING:
> +		btm = kzalloc(sizeof(*btm), GFP_KERNEL);
Check for !btm.

> +		btm->module = module;
> +		list_add(&btm->list, &bpf_trace_modules);
> +		break;
> +	case MODULE_STATE_GOING:
> +		list_for_each_entry_safe(btm, tmp, &bpf_trace_modules, list) {
Is the "_safe" version needed?

> +			if (btm->module == module) {
> +				list_del(&btm->list);
> +				kfree(btm);
> +				break;
> +			}
> +		}
> +		break;
> +	}
> +
> +	mutex_unlock(&bpf_module_mutex);
> +
> +	return 0;
> +}
> +
> +static struct notifier_block bpf_module_nb = {
> +	.notifier_call = bpf_event_notify,
> +};
> +
> +int __init bpf_event_init(void)
> +{
> +	register_module_notifier(&bpf_module_nb);
> +	return 0;
> +}
> +
> +fs_initcall(bpf_event_init);
> +#endif /* CONFIG_MODULES */
> -- 
> 2.17.1
> 

^ permalink raw reply

* [PATCHv2 net-next 0/3] sctp: add support for sk_reuseport
From: Xin Long @ 2018-11-12 10:27 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem

sctp sk_reuseport allows multiple socks to listen on the same port and
addresses, as long as these socks have the same uid. This works pretty
much as TCP/UDP does, the only difference is that sctp is multi-homing
and all the bind_addrs in these socks will have to completely matched,
otherwise listen() will return err.

The below is when 5 sockets are listening on 172.16.254.254:6400 on a
server, 26 sockets on a client connect to 172.16.254.254:6400 and each
may be processed by a different socket on the server which is selected
by hash(lport, pport, paddr) in reuseport_select_sock():

 # ss --sctp -nn
   State      Recv-Q Send-Q        Local Address:Port     Peer Address:Port
   LISTEN     0      10           172.16.254.254:6400                *:*
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.2.1:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.2.4:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.3.3:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.3.4:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.5.2:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.5.3:1234
   LISTEN     0      10           172.16.254.254:6400                *:*
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.1.3:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.1.4:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.3.2:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.4.1:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.4.2:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.4.3:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.4.4:1234
   LISTEN     0      10           172.16.254.254:6400                *:*
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.1.2:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.3.5:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.4.5:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400   172.16.253.253:1234
   LISTEN     0      10           172.16.254.254:6400                *:*
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.2.2:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.2.3:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.5.4:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.5.5:1234
   LISTEN     0      10           172.16.254.254:6400                *:*
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.1.1:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.1.5:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.2.5:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.3.1:1234
   `- ESTAB   0      0       172.16.254.254%eth1:6400       172.16.5.1:1234

Xin Long (3):
  sctp: do reuseport_select_sock in __sctp_rcv_lookup_endpoint
  sctp: add sock_reuseport for the sock in __sctp_hash_endpoint
  sctp: process sk_reuseport in sctp_get_port_local

 include/net/sctp/sctp.h    |   2 +-
 include/net/sctp/structs.h |   6 ++-
 net/core/sock_reuseport.c  |   1 +
 net/sctp/bind_addr.c       |  28 ++++++++++
 net/sctp/input.c           | 129 ++++++++++++++++++++++++++++++++-------------
 net/sctp/socket.c          |  49 +++++++++++------
 6 files changed, 162 insertions(+), 53 deletions(-)

-- 
2.1.0

^ permalink raw reply

* [PATCHv2 net-next 1/3] sctp: do reuseport_select_sock in __sctp_rcv_lookup_endpoint
From: Xin Long @ 2018-11-12 10:27 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1542018324.git.lucien.xin@gmail.com>

This is a part of sk_reuseport support for sctp, and it selects a
sock by the hashkey of lport, paddr and dport by default. It will
work until sk_reuseport support is added in sctp_get_port_local()
in the next patch.

v1->v2:
  - define lport as __be16 instead of __be32 as Marcelo pointed in
    __sctp_rcv_lookup_endpoint().

Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/input.c | 69 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index 7ab08a5..00f995e 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -57,6 +57,7 @@
 #include <net/sctp/checksum.h>
 #include <net/net_namespace.h>
 #include <linux/rhashtable.h>
+#include <net/sock_reuseport.h>
 
 /* Forward declarations for internal helpers. */
 static int sctp_rcv_ootb(struct sk_buff *);
@@ -65,8 +66,10 @@ static struct sctp_association *__sctp_rcv_lookup(struct net *net,
 				      const union sctp_addr *paddr,
 				      const union sctp_addr *laddr,
 				      struct sctp_transport **transportp);
-static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,
-						const union sctp_addr *laddr);
+static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(
+					struct net *net, struct sk_buff *skb,
+					const union sctp_addr *laddr,
+					const union sctp_addr *daddr);
 static struct sctp_association *__sctp_lookup_association(
 					struct net *net,
 					const union sctp_addr *local,
@@ -171,7 +174,7 @@ int sctp_rcv(struct sk_buff *skb)
 	asoc = __sctp_rcv_lookup(net, skb, &src, &dest, &transport);
 
 	if (!asoc)
-		ep = __sctp_rcv_lookup_endpoint(net, &dest);
+		ep = __sctp_rcv_lookup_endpoint(net, skb, &dest, &src);
 
 	/* Retrieve the common input handling substructure. */
 	rcvr = asoc ? &asoc->base : &ep->base;
@@ -771,16 +774,35 @@ void sctp_unhash_endpoint(struct sctp_endpoint *ep)
 	local_bh_enable();
 }
 
+static inline __u32 sctp_hashfn(const struct net *net, __be16 lport,
+				const union sctp_addr *paddr, __u32 seed)
+{
+	__u32 addr;
+
+	if (paddr->sa.sa_family == AF_INET6)
+		addr = jhash(&paddr->v6.sin6_addr, 16, seed);
+	else
+		addr = (__force __u32)paddr->v4.sin_addr.s_addr;
+
+	return  jhash_3words(addr, ((__force __u32)paddr->v4.sin_port) << 16 |
+			     (__force __u32)lport, net_hash_mix(net), seed);
+}
+
 /* Look up an endpoint. */
-static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,
-						const union sctp_addr *laddr)
+static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(
+					struct net *net, struct sk_buff *skb,
+					const union sctp_addr *laddr,
+					const union sctp_addr *paddr)
 {
 	struct sctp_hashbucket *head;
 	struct sctp_ep_common *epb;
 	struct sctp_endpoint *ep;
+	struct sock *sk;
+	__be16 lport;
 	int hash;
 
-	hash = sctp_ep_hashfn(net, ntohs(laddr->v4.sin_port));
+	lport = laddr->v4.sin_port;
+	hash = sctp_ep_hashfn(net, ntohs(lport));
 	head = &sctp_ep_hashtable[hash];
 	read_lock(&head->lock);
 	sctp_for_each_hentry(epb, &head->chain) {
@@ -792,6 +814,15 @@ static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,
 	ep = sctp_sk(net->sctp.ctl_sock)->ep;
 
 hit:
+	sk = ep->base.sk;
+	if (sk->sk_reuseport) {
+		__u32 phash = sctp_hashfn(net, lport, paddr, 0);
+
+		sk = reuseport_select_sock(sk, phash, skb,
+					   sizeof(struct sctphdr));
+		if (sk)
+			ep = sctp_sk(sk)->ep;
+	}
 	sctp_endpoint_hold(ep);
 	read_unlock(&head->lock);
 	return ep;
@@ -830,35 +861,17 @@ static inline int sctp_hash_cmp(struct rhashtable_compare_arg *arg,
 static inline __u32 sctp_hash_obj(const void *data, u32 len, u32 seed)
 {
 	const struct sctp_transport *t = data;
-	const union sctp_addr *paddr = &t->ipaddr;
-	const struct net *net = sock_net(t->asoc->base.sk);
-	__be16 lport = htons(t->asoc->base.bind_addr.port);
-	__u32 addr;
-
-	if (paddr->sa.sa_family == AF_INET6)
-		addr = jhash(&paddr->v6.sin6_addr, 16, seed);
-	else
-		addr = (__force __u32)paddr->v4.sin_addr.s_addr;
 
-	return  jhash_3words(addr, ((__force __u32)paddr->v4.sin_port) << 16 |
-			     (__force __u32)lport, net_hash_mix(net), seed);
+	return sctp_hashfn(sock_net(t->asoc->base.sk),
+			   htons(t->asoc->base.bind_addr.port),
+			   &t->ipaddr, seed);
 }
 
 static inline __u32 sctp_hash_key(const void *data, u32 len, u32 seed)
 {
 	const struct sctp_hash_cmp_arg *x = data;
-	const union sctp_addr *paddr = x->paddr;
-	const struct net *net = x->net;
-	__be16 lport = x->lport;
-	__u32 addr;
-
-	if (paddr->sa.sa_family == AF_INET6)
-		addr = jhash(&paddr->v6.sin6_addr, 16, seed);
-	else
-		addr = (__force __u32)paddr->v4.sin_addr.s_addr;
 
-	return  jhash_3words(addr, ((__force __u32)paddr->v4.sin_port) << 16 |
-			     (__force __u32)lport, net_hash_mix(net), seed);
+	return sctp_hashfn(x->net, x->lport, x->paddr, seed);
 }
 
 static const struct rhashtable_params sctp_hash_params = {
-- 
2.1.0

^ permalink raw reply related

* [PATCHv2 net-next 2/3] sctp: add sock_reuseport for the sock in __sctp_hash_endpoint
From: Xin Long @ 2018-11-12 10:27 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1542018324.git.lucien.xin@gmail.com>

This is a part of sk_reuseport support for sctp. It defines a helper
sctp_bind_addrs_check() to check if the bind_addrs in two socks are
matched. It will add sock_reuseport if they are completely matched,
and return err if they are partly matched, and alloc sock_reuseport
if all socks are not matched at all.

It will work until sk_reuseport support is added in
sctp_get_port_local() in the next patch.

v1->v2:
  - use 'laddr->valid && laddr2->valid' check instead as Marcelo
    pointed in sctp_bind_addrs_check().

Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/sctp.h    |  2 +-
 include/net/sctp/structs.h |  2 ++
 net/core/sock_reuseport.c  |  1 +
 net/sctp/bind_addr.c       | 28 ++++++++++++++++++++++
 net/sctp/input.c           | 60 +++++++++++++++++++++++++++++++++++++++-------
 net/sctp/socket.c          |  3 +--
 6 files changed, 85 insertions(+), 11 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 9a3b48a..cdf2e80 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -152,7 +152,7 @@ int sctp_primitive_RECONF(struct net *net, struct sctp_association *asoc,
  */
 int sctp_rcv(struct sk_buff *skb);
 int sctp_v4_err(struct sk_buff *skb, u32 info);
-void sctp_hash_endpoint(struct sctp_endpoint *);
+int sctp_hash_endpoint(struct sctp_endpoint *ep);
 void sctp_unhash_endpoint(struct sctp_endpoint *);
 struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *,
 			     struct sctphdr *, struct sctp_association **,
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index a11f937..15d017f 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1190,6 +1190,8 @@ int sctp_bind_addr_conflict(struct sctp_bind_addr *, const union sctp_addr *,
 			 struct sctp_sock *, struct sctp_sock *);
 int sctp_bind_addr_state(const struct sctp_bind_addr *bp,
 			 const union sctp_addr *addr);
+int sctp_bind_addrs_check(struct sctp_sock *sp,
+			  struct sctp_sock *sp2, int cnt2);
 union sctp_addr *sctp_find_unmatch_addr(struct sctp_bind_addr	*bp,
 					const union sctp_addr	*addrs,
 					int			addrcnt,
diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index ba5cba5..d8fe3e5 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -187,6 +187,7 @@ int reuseport_add_sock(struct sock *sk, struct sock *sk2, bool bind_inany)
 		call_rcu(&old_reuse->rcu, reuseport_free_rcu);
 	return 0;
 }
+EXPORT_SYMBOL(reuseport_add_sock);
 
 void reuseport_detach_sock(struct sock *sk)
 {
diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index 7df3704..ebf28ad 100644
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -337,6 +337,34 @@ int sctp_bind_addr_match(struct sctp_bind_addr *bp,
 	return match;
 }
 
+int sctp_bind_addrs_check(struct sctp_sock *sp,
+			  struct sctp_sock *sp2, int cnt2)
+{
+	struct sctp_bind_addr *bp2 = &sp2->ep->base.bind_addr;
+	struct sctp_bind_addr *bp = &sp->ep->base.bind_addr;
+	struct sctp_sockaddr_entry *laddr, *laddr2;
+	bool exist = false;
+	int cnt = 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(laddr, &bp->address_list, list) {
+		list_for_each_entry_rcu(laddr2, &bp2->address_list, list) {
+			if (sp->pf->af->cmp_addr(&laddr->a, &laddr2->a) &&
+			    laddr->valid && laddr2->valid) {
+				exist = true;
+				goto next;
+			}
+		}
+		cnt = 0;
+		break;
+next:
+		cnt++;
+	}
+	rcu_read_unlock();
+
+	return (cnt == cnt2) ? 0 : (exist ? -EEXIST : 1);
+}
+
 /* Does the address 'addr' conflict with any addresses in
  * the bp.
  */
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 00f995e..d7a649d 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -724,43 +724,87 @@ static int sctp_rcv_ootb(struct sk_buff *skb)
 }
 
 /* Insert endpoint into the hash table.  */
-static void __sctp_hash_endpoint(struct sctp_endpoint *ep)
+static int __sctp_hash_endpoint(struct sctp_endpoint *ep)
 {
-	struct net *net = sock_net(ep->base.sk);
-	struct sctp_ep_common *epb;
+	struct sock *sk = ep->base.sk;
+	struct net *net = sock_net(sk);
 	struct sctp_hashbucket *head;
+	struct sctp_ep_common *epb;
 
 	epb = &ep->base;
-
 	epb->hashent = sctp_ep_hashfn(net, epb->bind_addr.port);
 	head = &sctp_ep_hashtable[epb->hashent];
 
+	if (sk->sk_reuseport) {
+		bool any = sctp_is_ep_boundall(sk);
+		struct sctp_ep_common *epb2;
+		struct list_head *list;
+		int cnt = 0, err = 1;
+
+		list_for_each(list, &ep->base.bind_addr.address_list)
+			cnt++;
+
+		sctp_for_each_hentry(epb2, &head->chain) {
+			struct sock *sk2 = epb2->sk;
+
+			if (!net_eq(sock_net(sk2), net) || sk2 == sk ||
+			    !uid_eq(sock_i_uid(sk2), sock_i_uid(sk)) ||
+			    !sk2->sk_reuseport)
+				continue;
+
+			err = sctp_bind_addrs_check(sctp_sk(sk2),
+						    sctp_sk(sk), cnt);
+			if (!err) {
+				err = reuseport_add_sock(sk, sk2, any);
+				if (err)
+					return err;
+				break;
+			} else if (err < 0) {
+				return err;
+			}
+		}
+
+		if (err) {
+			err = reuseport_alloc(sk, any);
+			if (err)
+				return err;
+		}
+	}
+
 	write_lock(&head->lock);
 	hlist_add_head(&epb->node, &head->chain);
 	write_unlock(&head->lock);
+	return 0;
 }
 
 /* Add an endpoint to the hash. Local BH-safe. */
-void sctp_hash_endpoint(struct sctp_endpoint *ep)
+int sctp_hash_endpoint(struct sctp_endpoint *ep)
 {
+	int err;
+
 	local_bh_disable();
-	__sctp_hash_endpoint(ep);
+	err = __sctp_hash_endpoint(ep);
 	local_bh_enable();
+
+	return err;
 }
 
 /* Remove endpoint from the hash table.  */
 static void __sctp_unhash_endpoint(struct sctp_endpoint *ep)
 {
-	struct net *net = sock_net(ep->base.sk);
+	struct sock *sk = ep->base.sk;
 	struct sctp_hashbucket *head;
 	struct sctp_ep_common *epb;
 
 	epb = &ep->base;
 
-	epb->hashent = sctp_ep_hashfn(net, epb->bind_addr.port);
+	epb->hashent = sctp_ep_hashfn(sock_net(sk), epb->bind_addr.port);
 
 	head = &sctp_ep_hashtable[epb->hashent];
 
+	if (rcu_access_pointer(sk->sk_reuseport_cb))
+		reuseport_detach_sock(sk);
+
 	write_lock(&head->lock);
 	hlist_del_init(&epb->node);
 	write_unlock(&head->lock);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 739f3e5..2e955f1 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -7852,8 +7852,7 @@ static int sctp_listen_start(struct sock *sk, int backlog)
 	}
 
 	sk->sk_max_ack_backlog = backlog;
-	sctp_hash_endpoint(ep);
-	return 0;
+	return sctp_hash_endpoint(ep);
 }
 
 /*
-- 
2.1.0

^ permalink raw reply related

* [PATCHv2 net-next 3/3] sctp: process sk_reuseport in sctp_get_port_local
From: Xin Long @ 2018-11-12 10:27 UTC (permalink / raw)
  To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1542018324.git.lucien.xin@gmail.com>

When socks' sk_reuseport is set, the same port and address are allowed
to be bound into these socks who have the same uid.

Note that the difference from sk_reuse is that it allows multiple socks
to listen on the same port and address.

Acked-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h |  4 +++-
 net/sctp/socket.c          | 46 +++++++++++++++++++++++++++++++++-------------
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 15d017f..af9d494 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -96,7 +96,9 @@ struct sctp_stream;
 
 struct sctp_bind_bucket {
 	unsigned short	port;
-	unsigned short	fastreuse;
+	signed char	fastreuse;
+	signed char	fastreuseport;
+	kuid_t		fastuid;
 	struct hlist_node	node;
 	struct hlist_head	owner;
 	struct net	*net;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2e955f1..5299add 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -7644,8 +7644,10 @@ static struct sctp_bind_bucket *sctp_bucket_create(
 
 static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 {
-	bool reuse = (sk->sk_reuse || sctp_sk(sk)->reuse);
+	struct sctp_sock *sp = sctp_sk(sk);
+	bool reuse = (sk->sk_reuse || sp->reuse);
 	struct sctp_bind_hashbucket *head; /* hash list */
+	kuid_t uid = sock_i_uid(sk);
 	struct sctp_bind_bucket *pp;
 	unsigned short snum;
 	int ret;
@@ -7721,7 +7723,10 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 
 		pr_debug("%s: found a possible match\n", __func__);
 
-		if (pp->fastreuse && reuse && sk->sk_state != SCTP_SS_LISTENING)
+		if ((pp->fastreuse && reuse &&
+		     sk->sk_state != SCTP_SS_LISTENING) ||
+		    (pp->fastreuseport && sk->sk_reuseport &&
+		     uid_eq(pp->fastuid, uid)))
 			goto success;
 
 		/* Run through the list of sockets bound to the port
@@ -7735,16 +7740,18 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 		 * in an endpoint.
 		 */
 		sk_for_each_bound(sk2, &pp->owner) {
-			struct sctp_endpoint *ep2;
-			ep2 = sctp_sk(sk2)->ep;
+			struct sctp_sock *sp2 = sctp_sk(sk2);
+			struct sctp_endpoint *ep2 = sp2->ep;
 
 			if (sk == sk2 ||
-			    (reuse && (sk2->sk_reuse || sctp_sk(sk2)->reuse) &&
-			     sk2->sk_state != SCTP_SS_LISTENING))
+			    (reuse && (sk2->sk_reuse || sp2->reuse) &&
+			     sk2->sk_state != SCTP_SS_LISTENING) ||
+			    (sk->sk_reuseport && sk2->sk_reuseport &&
+			     uid_eq(uid, sock_i_uid(sk2))))
 				continue;
 
-			if (sctp_bind_addr_conflict(&ep2->base.bind_addr, addr,
-						 sctp_sk(sk2), sctp_sk(sk))) {
+			if (sctp_bind_addr_conflict(&ep2->base.bind_addr,
+						    addr, sp2, sp)) {
 				ret = (long)sk2;
 				goto fail_unlock;
 			}
@@ -7767,19 +7774,32 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 			pp->fastreuse = 1;
 		else
 			pp->fastreuse = 0;
-	} else if (pp->fastreuse &&
-		   (!reuse || sk->sk_state == SCTP_SS_LISTENING))
-		pp->fastreuse = 0;
+
+		if (sk->sk_reuseport) {
+			pp->fastreuseport = 1;
+			pp->fastuid = uid;
+		} else {
+			pp->fastreuseport = 0;
+		}
+	} else {
+		if (pp->fastreuse &&
+		    (!reuse || sk->sk_state == SCTP_SS_LISTENING))
+			pp->fastreuse = 0;
+
+		if (pp->fastreuseport &&
+		    (!sk->sk_reuseport || !uid_eq(pp->fastuid, uid)))
+			pp->fastreuseport = 0;
+	}
 
 	/* We are set, so fill up all the data in the hash table
 	 * entry, tie the socket list information with the rest of the
 	 * sockets FIXME: Blurry, NPI (ipg).
 	 */
 success:
-	if (!sctp_sk(sk)->bind_hash) {
+	if (!sp->bind_hash) {
 		inet_sk(sk)->inet_num = snum;
 		sk_add_bind_node(sk, &pp->owner);
-		sctp_sk(sk)->bind_hash = pp;
+		sp->bind_hash = pp;
 	}
 	ret = 0;
 
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH 08/20] octeontx2-af: Alloc and config NPC MCAM entry at a time
From: Sunil Kovvuri @ 2018-11-12 10:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux Netdev List, David S. Miller, linux-soc, Sunil Goutham
In-Reply-To: <CAK8P3a1O6Ektz-=Th4pmPwk5K7=AZCQUHaj3KYf_JG_XP1x2bA@mail.gmail.com>

On Sat, Nov 10, 2018 at 2:36 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, Nov 9, 2018 at 6:13 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> > On Fri, Nov 9, 2018 at 4:32 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Fri, Nov 9, 2018 at 5:21 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
>
> > >
> > > Since b is aligned to four bytes, you get padding between a and b.
> > > On top of that, you also get padding after c to make the size of
> > > structure itself be a multiple of its alignment. For interfaces, we
> > > should avoid both kinds of padding. This can be done by marking
> > > members as __packed (usually I don't recommend that), by
> > > changing the size of members, or by adding explicit 'reserved'
> > > fields in place of the padding.
> > >
> > > > > I also noticed a similar problem in struct mbox_msghdr. Maybe
> > > > > use the 'pahole' tool to check for this kind of padding in the
> > > > > API structures.
> >
> > Got your point now and agree that padding has to be avoided.
> > But this is a big change and above pointed structure is not
> > the only one as this applies to all structures in the file.
> >
> > Would it be okay if I submit a separate patch after this series
> > addressing all structures ?
>
> It depends on how you want to address it. If you want to
> change the structure layout, then I think it would be better
> integrated into the series as that is an incompatible interface
> change. If you just want to add reserved members to make
> the padding explicit, that could be a follow-up.
>
>         Arnd

Upon further thought i think padding is needed here but adding reserved fields
is also not enough. Probably i will have to align all struct elements to 64bit
with  __attribute__ ((aligned(8))) to avoid unaligned access faults when device
is attached to guest. VFIO maps BARs as device memory, so irrespective of
whatever mapping i do in driver, reads/writes to the mailbox region might result
in unaligned access fault.

Checking if there are other ways of solving it.

Thanks,
Sunil.

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Martin Blumenstingl @ 2018-11-12 20:32 UTC (permalink / raw)
  To: hkallweit1
  Cc: f.fainelli, andrew, netdev, richardcochran, linux-kernel,
	bcm-kernel-feedback-list, khilman, carlo, linux-amlogic, davem,
	linux-arm-kernel
In-Reply-To: <c067d321-c69e-706d-304a-04e1e477b47c@gmail.com>

Hi Heiner,

On Fri, Nov 9, 2018 at 9:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 09.11.2018 21:33, Florian Fainelli wrote:
> > On 11/9/18 12:22 PM, Heiner Kallweit wrote:
> >> On 09.11.2018 21:13, Andrew Lunn wrote:
> >>> Hi Heiner
> >>>
> >>>> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> >>>> +{
> >>>> +  return phydrv->config_intr || phydrv->ack_interrupt;
> >>>> +}
> >>>
> >>> Should this be && not || ? I thought both needed to be provided for
> >>> interrupts to work.
> >>>
> >>>        Andrew
> >>>
> >> I've seen at least one driver which configures interrupts in
> >> config_init and doesn't define a config_intr callback
> >> (ack_interrupt callback is there)
> >
> > That driver should probably be fixed, while it most likely does not make
> > any significant difference during probe/connect, since config_init() and
> > config_intr() are virtually happening at the same time, this is not
> > necessarily true when disconnecting from the PHY where we really want
> > config_intr() to effectively disable the interrupts and not leaving
> > something enabled that would now become unmaskable, because no more
> > driver attached.
> >
> Found the driver: It's the IP101A/G in icplus.c
> It should be easy to fix the behavior and move the interrupt config
> to a config_intr callback. But the last real changes to the driver
> have been done 6 years ago, so I'm not sure there's anybody out
> there who can test.
if you want I can take care of the IP101A/G code.
I have at least one board with an IP101A/G (PHY ID: 0x02430c54,
according to the schematics it's an IP101GR-GP) where the interrupt is
routed to the SoC.

please let me know whether you'd like to work on it or if I should
give it a try.


Regards
Martin

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Heiner Kallweit @ 2018-11-12 20:38 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: f.fainelli, andrew, netdev, richardcochran, linux-kernel,
	bcm-kernel-feedback-list, khilman, carlo, linux-amlogic, davem,
	linux-arm-kernel
In-Reply-To: <CAFBinCCmNYGp8kzy45ojjr27tEaPVs-QwMsLRrfXHCvw_taDOw@mail.gmail.com>

On 12.11.2018 21:32, Martin Blumenstingl wrote:
> Hi Heiner,
> 
> On Fri, Nov 9, 2018 at 9:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 09.11.2018 21:33, Florian Fainelli wrote:
>>> On 11/9/18 12:22 PM, Heiner Kallweit wrote:
>>>> On 09.11.2018 21:13, Andrew Lunn wrote:
>>>>> Hi Heiner
>>>>>
>>>>>> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
>>>>>> +{
>>>>>> +  return phydrv->config_intr || phydrv->ack_interrupt;
>>>>>> +}
>>>>>
>>>>> Should this be && not || ? I thought both needed to be provided for
>>>>> interrupts to work.
>>>>>
>>>>>        Andrew
>>>>>
>>>> I've seen at least one driver which configures interrupts in
>>>> config_init and doesn't define a config_intr callback
>>>> (ack_interrupt callback is there)
>>>
>>> That driver should probably be fixed, while it most likely does not make
>>> any significant difference during probe/connect, since config_init() and
>>> config_intr() are virtually happening at the same time, this is not
>>> necessarily true when disconnecting from the PHY where we really want
>>> config_intr() to effectively disable the interrupts and not leaving
>>> something enabled that would now become unmaskable, because no more
>>> driver attached.
>>>
>> Found the driver: It's the IP101A/G in icplus.c
>> It should be easy to fix the behavior and move the interrupt config
>> to a config_intr callback. But the last real changes to the driver
>> have been done 6 years ago, so I'm not sure there's anybody out
>> there who can test.
> if you want I can take care of the IP101A/G code.
> I have at least one board with an IP101A/G (PHY ID: 0x02430c54,
> according to the schematics it's an IP101GR-GP) where the interrupt is
> routed to the SoC.
> 
> please let me know whether you'd like to work on it or if I should
> give it a try.
> 
I made the change already based on the datasheet of IP101A LF which
is supposed to be register-compatible with IP101A/G.
The patch is not applied yet, you can find it in the mailing
list archive or in patchwork. Would be great if you could test it
and report problems or add a Tested-by.

> 
> Regards
> Martin
> 
Rgds, Heiner

^ permalink raw reply

* [PATCH net v2 0/4] qed: Miscellaneous bug fixes
From: Denis Bolotin @ 2018-11-12 10:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: michal.kalderon, ariel.elior, Denis Bolotin

Hi Dave,

This patch series fixes several unrelated bugs across the driver.
Please consider applying to net.

V1->V2:
-------
Use dma_rmb() instead of rmb().

Thanks,
Denis

Denis Bolotin (3):
  qed: Fix PTT leak in qed_drain()
  qed: Fix overriding offload_tc by protocols without APP TLV
  qed: Fix reading wrong value in loop condition

Michal Kalderon (1):
  qed: Fix rdma_info structure allocation

 drivers/net/ethernet/qlogic/qed/qed_dcbx.c | 14 ++++-----
 drivers/net/ethernet/qlogic/qed/qed_dev.c  | 15 ++++++---
 drivers/net/ethernet/qlogic/qed/qed_int.c  |  2 ++
 drivers/net/ethernet/qlogic/qed/qed_main.c |  2 +-
 drivers/net/ethernet/qlogic/qed/qed_rdma.c | 50 +++++++++++++++++-------------
 drivers/net/ethernet/qlogic/qed/qed_rdma.h |  5 +++
 6 files changed, 55 insertions(+), 33 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH net v2 1/4] qed: Fix PTT leak in qed_drain()
From: Denis Bolotin @ 2018-11-12 10:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: michal.kalderon, ariel.elior, Denis Bolotin
In-Reply-To: <20181112105023.23842-1-denis.bolotin@cavium.com>

Release PTT before entering error flow.

Signed-off-by: Denis Bolotin <denis.bolotin@cavium.com>
Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 35fd0db..fff7f04 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1782,9 +1782,9 @@ static int qed_drain(struct qed_dev *cdev)
 			return -EBUSY;
 		}
 		rc = qed_mcp_drain(hwfn, ptt);
+		qed_ptt_release(hwfn, ptt);
 		if (rc)
 			return rc;
-		qed_ptt_release(hwfn, ptt);
 	}
 
 	return 0;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net v2 4/4] qed: Fix reading wrong value in loop condition
From: Denis Bolotin @ 2018-11-12 10:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: michal.kalderon, ariel.elior, Denis Bolotin
In-Reply-To: <20181112105023.23842-1-denis.bolotin@cavium.com>

The value of "sb_index" is written by the hardware. Reading its value and
writing it to "index" must finish before checking the loop condition.

Signed-off-by: Denis Bolotin <denis.bolotin@cavium.com>
Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_int.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.c b/drivers/net/ethernet/qlogic/qed/qed_int.c
index 0f0aba7..b22f464 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_int.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_int.c
@@ -992,6 +992,8 @@ static int qed_int_attentions(struct qed_hwfn *p_hwfn)
 	 */
 	do {
 		index = p_sb_attn->sb_index;
+		/* finish reading index before the loop condition */
+		dma_rmb();
 		attn_bits = le32_to_cpu(p_sb_attn->atten_bits);
 		attn_acks = le32_to_cpu(p_sb_attn->atten_ack);
 	} while (index != p_sb_attn->sb_index);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net v2 3/4] qed: Fix rdma_info structure allocation
From: Denis Bolotin @ 2018-11-12 10:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: michal.kalderon, ariel.elior, Denis Bolotin
In-Reply-To: <20181112105023.23842-1-denis.bolotin@cavium.com>

From: Michal Kalderon <michal.kalderon@cavium.com>

Certain flows need to access the rdma-info structure, for example dcbx
update flows. In some cases there can be a race between the allocation or
deallocation of the structure which was done in roce start / roce stop and
an asynchrounous dcbx event that tries to access the structure.
For this reason, we move the allocation of the rdma_info structure to be
similar to the iscsi/fcoe info structures which are allocated during device
setup.
We add a new field of "active" to the struct to define whether roce has
already been started or not, and this is checked instead of whether the
pointer to the info structure.

Fixes: 51ff17251c9c ("qed: Add support for RoCE hw init")
Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
Signed-off-by: Denis Bolotin <denis.bolotin@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_dev.c  | 15 ++++++---
 drivers/net/ethernet/qlogic/qed/qed_rdma.c | 50 +++++++++++++++++-------------
 drivers/net/ethernet/qlogic/qed/qed_rdma.h |  5 +++
 3 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 7ceb2b9..cff1410 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -185,6 +185,10 @@ void qed_resc_free(struct qed_dev *cdev)
 			qed_iscsi_free(p_hwfn);
 			qed_ooo_free(p_hwfn);
 		}
+
+		if (QED_IS_RDMA_PERSONALITY(p_hwfn))
+			qed_rdma_info_free(p_hwfn);
+
 		qed_iov_free(p_hwfn);
 		qed_l2_free(p_hwfn);
 		qed_dmae_info_free(p_hwfn);
@@ -1081,6 +1085,12 @@ int qed_resc_alloc(struct qed_dev *cdev)
 				goto alloc_err;
 		}
 
+		if (QED_IS_RDMA_PERSONALITY(p_hwfn)) {
+			rc = qed_rdma_info_alloc(p_hwfn);
+			if (rc)
+				goto alloc_err;
+		}
+
 		/* DMA info initialization */
 		rc = qed_dmae_info_alloc(p_hwfn);
 		if (rc)
@@ -2102,11 +2112,8 @@ int qed_hw_start_fastpath(struct qed_hwfn *p_hwfn)
 	if (!p_ptt)
 		return -EAGAIN;
 
-	/* If roce info is allocated it means roce is initialized and should
-	 * be enabled in searcher.
-	 */
 	if (p_hwfn->p_rdma_info &&
-	    p_hwfn->b_rdma_enabled_in_prs)
+	    p_hwfn->p_rdma_info->active && p_hwfn->b_rdma_enabled_in_prs)
 		qed_wr(p_hwfn, p_ptt, p_hwfn->rdma_prs_search_reg, 0x1);
 
 	/* Re-open incoming traffic */
diff --git a/drivers/net/ethernet/qlogic/qed/qed_rdma.c b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
index 6211343..7873d6df 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
@@ -140,22 +140,34 @@ static u32 qed_rdma_get_sb_id(void *p_hwfn, u32 rel_sb_id)
 	return FEAT_NUM((struct qed_hwfn *)p_hwfn, QED_PF_L2_QUE) + rel_sb_id;
 }
 
-static int qed_rdma_alloc(struct qed_hwfn *p_hwfn,
-			  struct qed_ptt *p_ptt,
-			  struct qed_rdma_start_in_params *params)
+int qed_rdma_info_alloc(struct qed_hwfn *p_hwfn)
 {
 	struct qed_rdma_info *p_rdma_info;
-	u32 num_cons, num_tasks;
-	int rc = -ENOMEM;
 
-	DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Allocating RDMA\n");
-
-	/* Allocate a struct with current pf rdma info */
 	p_rdma_info = kzalloc(sizeof(*p_rdma_info), GFP_KERNEL);
 	if (!p_rdma_info)
-		return rc;
+		return -ENOMEM;
+
+	spin_lock_init(&p_rdma_info->lock);
 
 	p_hwfn->p_rdma_info = p_rdma_info;
+	return 0;
+}
+
+void qed_rdma_info_free(struct qed_hwfn *p_hwfn)
+{
+	kfree(p_hwfn->p_rdma_info);
+	p_hwfn->p_rdma_info = NULL;
+}
+
+static int qed_rdma_alloc(struct qed_hwfn *p_hwfn)
+{
+	struct qed_rdma_info *p_rdma_info = p_hwfn->p_rdma_info;
+	u32 num_cons, num_tasks;
+	int rc = -ENOMEM;
+
+	DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Allocating RDMA\n");
+
 	if (QED_IS_IWARP_PERSONALITY(p_hwfn))
 		p_rdma_info->proto = PROTOCOLID_IWARP;
 	else
@@ -183,7 +195,7 @@ static int qed_rdma_alloc(struct qed_hwfn *p_hwfn,
 	/* Allocate a struct with device params and fill it */
 	p_rdma_info->dev = kzalloc(sizeof(*p_rdma_info->dev), GFP_KERNEL);
 	if (!p_rdma_info->dev)
-		goto free_rdma_info;
+		return rc;
 
 	/* Allocate a struct with port params and fill it */
 	p_rdma_info->port = kzalloc(sizeof(*p_rdma_info->port), GFP_KERNEL);
@@ -298,8 +310,6 @@ static int qed_rdma_alloc(struct qed_hwfn *p_hwfn,
 	kfree(p_rdma_info->port);
 free_rdma_dev:
 	kfree(p_rdma_info->dev);
-free_rdma_info:
-	kfree(p_rdma_info);
 
 	return rc;
 }
@@ -370,8 +380,6 @@ static void qed_rdma_resc_free(struct qed_hwfn *p_hwfn)
 
 	kfree(p_rdma_info->port);
 	kfree(p_rdma_info->dev);
-
-	kfree(p_rdma_info);
 }
 
 static void qed_rdma_free_tid(void *rdma_cxt, u32 itid)
@@ -679,8 +687,6 @@ static int qed_rdma_setup(struct qed_hwfn *p_hwfn,
 
 	DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "RDMA setup\n");
 
-	spin_lock_init(&p_hwfn->p_rdma_info->lock);
-
 	qed_rdma_init_devinfo(p_hwfn, params);
 	qed_rdma_init_port(p_hwfn);
 	qed_rdma_init_events(p_hwfn, params);
@@ -727,7 +733,7 @@ static int qed_rdma_stop(void *rdma_cxt)
 	/* Disable RoCE search */
 	qed_wr(p_hwfn, p_ptt, p_hwfn->rdma_prs_search_reg, 0);
 	p_hwfn->b_rdma_enabled_in_prs = false;
-
+	p_hwfn->p_rdma_info->active = 0;
 	qed_wr(p_hwfn, p_ptt, PRS_REG_ROCE_DEST_QP_MAX_PF, 0);
 
 	ll2_ethertype_en = qed_rd(p_hwfn, p_ptt, PRS_REG_LIGHT_L2_ETHERTYPE_EN);
@@ -1236,7 +1242,8 @@ static int qed_rdma_destroy_qp(void *rdma_cxt, struct qed_rdma_qp *qp)
 	u8 max_stats_queues;
 	int rc;
 
-	if (!rdma_cxt || !in_params || !out_params || !p_hwfn->p_rdma_info) {
+	if (!rdma_cxt || !in_params || !out_params ||
+	    !p_hwfn->p_rdma_info->active) {
 		DP_ERR(p_hwfn->cdev,
 		       "qed roce create qp failed due to NULL entry (rdma_cxt=%p, in=%p, out=%p, roce_info=?\n",
 		       rdma_cxt, in_params, out_params);
@@ -1802,8 +1809,8 @@ bool qed_rdma_allocated_qps(struct qed_hwfn *p_hwfn)
 {
 	bool result;
 
-	/* if rdma info has not been allocated, naturally there are no qps */
-	if (!p_hwfn->p_rdma_info)
+	/* if rdma wasn't activated yet, naturally there are no qps */
+	if (!p_hwfn->p_rdma_info->active)
 		return false;
 
 	spin_lock_bh(&p_hwfn->p_rdma_info->lock);
@@ -1849,7 +1856,7 @@ static int qed_rdma_start(void *rdma_cxt,
 	if (!p_ptt)
 		goto err;
 
-	rc = qed_rdma_alloc(p_hwfn, p_ptt, params);
+	rc = qed_rdma_alloc(p_hwfn);
 	if (rc)
 		goto err1;
 
@@ -1858,6 +1865,7 @@ static int qed_rdma_start(void *rdma_cxt,
 		goto err2;
 
 	qed_ptt_release(p_hwfn, p_ptt);
+	p_hwfn->p_rdma_info->active = 1;
 
 	return rc;
 
diff --git a/drivers/net/ethernet/qlogic/qed/qed_rdma.h b/drivers/net/ethernet/qlogic/qed/qed_rdma.h
index 6f722ee..50d609c 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_rdma.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_rdma.h
@@ -102,6 +102,7 @@ struct qed_rdma_info {
 	u16 max_queue_zones;
 	enum protocol_type proto;
 	struct qed_iwarp_info iwarp;
+	u8 active:1;
 };
 
 struct qed_rdma_qp {
@@ -176,10 +177,14 @@ struct qed_rdma_qp {
 #if IS_ENABLED(CONFIG_QED_RDMA)
 void qed_rdma_dpm_bar(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
 void qed_rdma_dpm_conf(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
+int qed_rdma_info_alloc(struct qed_hwfn *p_hwfn);
+void qed_rdma_info_free(struct qed_hwfn *p_hwfn);
 #else
 static inline void qed_rdma_dpm_conf(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) {}
 static inline void qed_rdma_dpm_bar(struct qed_hwfn *p_hwfn,
 				    struct qed_ptt *p_ptt) {}
+static inline int qed_rdma_info_alloc(struct qed_hwfn *p_hwfn) {return -EINVAL}
+static inline void qed_rdma_info_free(struct qed_hwfn *p_hwfn) {}
 #endif
 
 int
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net v2 2/4] qed: Fix overriding offload_tc by protocols without APP TLV
From: Denis Bolotin @ 2018-11-12 10:50 UTC (permalink / raw)
  To: davem, netdev; +Cc: michal.kalderon, ariel.elior, Denis Bolotin
In-Reply-To: <20181112105023.23842-1-denis.bolotin@cavium.com>

The TC received from APP TLV is stored in offload_tc, and should not be
set by protocols which did not receive an APP TLV. Fixed the condition
when overriding the offload_tc.

Signed-off-by: Denis Bolotin <denis.bolotin@cavium.com>
Signed-off-by: Michal Kalderon <michal.kalderon@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_dcbx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
index 8e8fa82..69966df 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dcbx.c
@@ -191,7 +191,7 @@ static bool qed_dcbx_roce_v2_tlv(u32 app_info_bitmap, u16 proto_id, bool ieee)
 static void
 qed_dcbx_set_params(struct qed_dcbx_results *p_data,
 		    struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
-		    bool enable, u8 prio, u8 tc,
+		    bool app_tlv, bool enable, u8 prio, u8 tc,
 		    enum dcbx_protocol_type type,
 		    enum qed_pci_personality personality)
 {
@@ -210,7 +210,7 @@ static bool qed_dcbx_roce_v2_tlv(u32 app_info_bitmap, u16 proto_id, bool ieee)
 		p_data->arr[type].dont_add_vlan0 = true;
 
 	/* QM reconf data */
-	if (p_hwfn->hw_info.personality == personality)
+	if (app_tlv && p_hwfn->hw_info.personality == personality)
 		qed_hw_info_set_offload_tc(&p_hwfn->hw_info, tc);
 
 	/* Configure dcbx vlan priority in doorbell block for roce EDPM */
@@ -225,7 +225,7 @@ static bool qed_dcbx_roce_v2_tlv(u32 app_info_bitmap, u16 proto_id, bool ieee)
 static void
 qed_dcbx_update_app_info(struct qed_dcbx_results *p_data,
 			 struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
-			 bool enable, u8 prio, u8 tc,
+			 bool app_tlv, bool enable, u8 prio, u8 tc,
 			 enum dcbx_protocol_type type)
 {
 	enum qed_pci_personality personality;
@@ -240,7 +240,7 @@ static bool qed_dcbx_roce_v2_tlv(u32 app_info_bitmap, u16 proto_id, bool ieee)
 
 		personality = qed_dcbx_app_update[i].personality;
 
-		qed_dcbx_set_params(p_data, p_hwfn, p_ptt, enable,
+		qed_dcbx_set_params(p_data, p_hwfn, p_ptt, app_tlv, enable,
 				    prio, tc, type, personality);
 	}
 }
@@ -319,8 +319,8 @@ static bool qed_dcbx_roce_v2_tlv(u32 app_info_bitmap, u16 proto_id, bool ieee)
 				enable = true;
 			}
 
-			qed_dcbx_update_app_info(p_data, p_hwfn, p_ptt, enable,
-						 priority, tc, type);
+			qed_dcbx_update_app_info(p_data, p_hwfn, p_ptt, true,
+						 enable, priority, tc, type);
 		}
 	}
 
@@ -341,7 +341,7 @@ static bool qed_dcbx_roce_v2_tlv(u32 app_info_bitmap, u16 proto_id, bool ieee)
 			continue;
 
 		enable = (type == DCBX_PROTOCOL_ETH) ? false : !!dcbx_version;
-		qed_dcbx_update_app_info(p_data, p_hwfn, p_ptt, enable,
+		qed_dcbx_update_app_info(p_data, p_hwfn, p_ptt, false, enable,
 					 priority, tc, type);
 	}
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] Bluetooth: Fix locking in bt_accept_enqueue() for BH context
From: Matthias Kaehlcke @ 2018-11-12 21:40 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, David S . Miller, Dean Jenkins
  Cc: linux-bluetooth, netdev, linux-kernel, Konstantin Khlebnikov,
	Balakrishna Godavarthi, Douglas Anderson, Dmitry Grinberg
In-Reply-To: <20181015223910.197729-1-mka@chromium.org>

On Mon, Oct 15, 2018 at 03:39:10PM -0700, Matthias Kaehlcke wrote:
> With commit e16337622016 ("Bluetooth: Handle bt_accept_enqueue() socket
> atomically") lock_sock[_nested]() is used to acquire the socket lock
> before manipulating the socket. lock_sock[_nested]() may block, which
> is problematic since bt_accept_enqueue() can be called in bottom half
> context (e.g. from rfcomm_connect_ind()).
> 
> The socket API provides bh_lock_sock[_nested]() to acquire the socket
> lock in bottom half context. Check the context in bt_accept_enqueue()
> and use the appropriate locking mechanism for the context.
> 
> Fixes: e16337622016 ("Bluetooth: Handle bt_accept_enqueue() socket atomically")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Not sure if this is the correct solution, it's certainly not elegant and
> checkpatch.pl complains that in_atomic() shouldn't be used outside of
> core kernel code. I'm open to other suggestions :)
> 
>  net/bluetooth/af_bluetooth.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index deacc52d7ff1..0f0540dbb44a 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -159,10 +159,20 @@ void bt_accept_enqueue(struct sock *parent, struct sock *sk)
>  	BT_DBG("parent %p, sk %p", parent, sk);
>  
>  	sock_hold(sk);
> -	lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
> +
> +	if (in_atomic())
> +		bh_lock_sock_nested(sk);
> +	else
> +		lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
> +
>  	list_add_tail(&bt_sk(sk)->accept_q, &bt_sk(parent)->accept_q);
>  	bt_sk(sk)->parent = parent;
> -	release_sock(sk);
> +
> +	if (in_atomic())
> +		bh_unlock_sock(sk);
> +	else
> +		release_sock(sk);
> +
>  	parent->sk_ack_backlog++;
>  }
>  EXPORT_SYMBOL(bt_accept_enqueue);

Any comments or ideas for a better solutions?

Thanks

Matthias

^ permalink raw reply

* pull-request: can 2018-11-09
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

this is a pull request of 20 patches for net/master.

First we have a patch by Oliver Hartkopp which changes the raw socket's
raw_sendmsg() to return an error value if the user tries to send a CANFD
frame to a CAN-2.0 device.

The next two patches are by Jimmy Assarsson and fix potential problems
in the kvaser_usb driver.

YueHaibing's patches for the ucan driver fix a compile time warning and
remove a duplicate include.

Eugeniu Rosca patch adds more binding documentation to the rcar_can
driver bindings. The next two patches are by Fabrizio Castro for the
rcar_can driver and fixes a problem in the driver's probe function and
document the r8a774a1 binding.

Lukas Wunner's patch fixes a recpetion problem in hi311x driver by
switching from edge to level triggered interruts.

The next three patches all target the flexcan driver. Pankaj Bansal's
patch unconditionally unlocks the last mailbox used for RX. Alexander
Stein provides a better workaround for a hardware limitation when
sending RTR frames, by using the last mailbox for TX, resulting in fewer
lost frames. The patch by me simplyfies the driver, by making a runtime
value a compile time constant.

The following 4 patches are by me and provide the groundwork for the
next patches by Oleksij Rempel. To avoid code duplication common code in
the common CAN driver infrastructure is factured out and error handling
is cleaned up.

The next 4 patches are by Oleksij Rempel and fix the problem in the
flexcan driver that other processes see TX frames arrive out of order
with ragards to a RX'ed frame (which are send by a different system on
the CAN bus as the result of our TX frame).

regards,
Marc

---

The following changes since commit 85b18b0237ce9986a81a1b9534b5e2ee116f5504:

  net: smsc95xx: Fix MTU range (2018-11-08 19:54:49 -0800)

are available in the Git repository at:

  ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.20-20181109

for you to fetch changes up to d788905f68fd4714c82936f6f7f1d3644d7ae7ef:

  can: flexcan: use can_rx_offload_queue_sorted() for flexcan_irq_bus_*() (2018-11-09 17:33:33 +0100)

----------------------------------------------------------------
linux-can-fixes-for-4.20-20181109

----------------------------------------------------------------
Alexander Stein (1):
      can: flexcan: Always use last mailbox for TX

Eugeniu Rosca (1):
      dt-bindings: can: rcar_can: document r8a77965 support

Fabrizio Castro (2):
      can: rcar_can: Fix erroneous registration
      dt-bindings: can: rcar_can: Add r8a774a1 support

Jimmy Assarsson (2):
      can: kvaser_usb: Fix potential uninitialized variable use
      can: kvaser_usb: Fix accessing freed memory in kvaser_usb_start_xmit()

Lukas Wunner (1):
      can: hi311x: Use level-triggered interrupt

Marc Kleine-Budde (5):
      can: flexcan: remove not needed struct flexcan_priv::tx_mb and struct flexcan_priv::tx_mb_idx
      can: dev: can_get_echo_skb(): factor out non sending code to __can_get_echo_skb()
      can: dev: __can_get_echo_skb(): replace struct can_frame by canfd_frame to access frame length
      can: dev: __can_get_echo_skb(): Don't crash the kernel if can_priv::echo_skb is accessed out of bounds
      can: dev: __can_get_echo_skb(): print error message, if trying to echo non existing skb

Oleksij Rempel (4):
      can: rx-offload: introduce can_rx_offload_get_echo_skb() and can_rx_offload_queue_sorted() functions
      can: flexcan: handle tx-complete CAN frames via rx-offload infrastructure
      can: rx-offload: rename can_rx_offload_irq_queue_err_skb() to can_rx_offload_queue_tail()
      can: flexcan: use can_rx_offload_queue_sorted() for flexcan_irq_bus_*()

Oliver Hartkopp (1):
      can: raw: check for CAN FD capable netdev in raw_sendmsg()

Pankaj Bansal (1):
      can: flexcan: Unlock the MB unconditionally

YueHaibing (2):
      can: ucan: remove set but not used variable 'udev'
      can: ucan: remove duplicated include from ucan.c

 .../devicetree/bindings/net/can/holt_hi311x.txt    |   2 +-
 .../devicetree/bindings/net/can/rcar_can.txt       |  28 ++++--
 drivers/net/can/dev.c                              |  48 ++++++---
 drivers/net/can/flexcan.c                          | 108 ++++++++++++---------
 drivers/net/can/rcar/rcar_can.c                    |   5 +-
 drivers/net/can/rx-offload.c                       |  51 +++++++++-
 drivers/net/can/spi/hi311x.c                       |   2 +-
 drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c   |   4 +-
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c  |  10 +-
 drivers/net/can/usb/ucan.c                         |   7 --
 include/linux/can/dev.h                            |   1 +
 include/linux/can/rx-offload.h                     |   7 +-
 net/can/raw.c                                      |  15 +--
 13 files changed, 190 insertions(+), 98 deletions(-)

^ permalink raw reply

* [PATCH 04/20] can: ucan: remove set but not used variable 'udev'
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, YueHaibing, Marc Kleine-Budde
In-Reply-To: <20181112115728.18331-1-mkl@pengutronix.de>

From: YueHaibing <yuehaibing@huawei.com>

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/can/usb/ucan.c: In function 'ucan_disconnect':
drivers/net/can/usb/ucan.c:1578:21: warning:
 variable 'udev' set but not used [-Wunused-but-set-variable]
  struct usb_device *udev;

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Reviewed-by: Martin Elshuber <martin.elshuber@theobroma-systems.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/ucan.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
index 0678a38b1af4..c9fd83e8d947 100644
--- a/drivers/net/can/usb/ucan.c
+++ b/drivers/net/can/usb/ucan.c
@@ -1575,11 +1575,8 @@ static int ucan_probe(struct usb_interface *intf,
 /* disconnect the device */
 static void ucan_disconnect(struct usb_interface *intf)
 {
-	struct usb_device *udev;
 	struct ucan_priv *up = usb_get_intfdata(intf);
 
-	udev = interface_to_usbdev(intf);
-
 	usb_set_intfdata(intf, NULL);
 
 	if (up) {
-- 
2.19.1

^ permalink raw reply related

* [PATCH 05/20] can: ucan: remove duplicated include from ucan.c
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, YueHaibing, Marc Kleine-Budde
In-Reply-To: <20181112115728.18331-1-mkl@pengutronix.de>

From: YueHaibing <yuehaibing@huawei.com>

Remove duplicated include.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Reviewed-by: Martin Elshuber <martin.elshuber@theobroma-systems.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/ucan.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c
index c9fd83e8d947..f3d5bda012a1 100644
--- a/drivers/net/can/usb/ucan.c
+++ b/drivers/net/can/usb/ucan.c
@@ -35,10 +35,6 @@
 #include <linux/slab.h>
 #include <linux/usb.h>
 
-#include <linux/can.h>
-#include <linux/can/dev.h>
-#include <linux/can/error.h>
-
 #define UCAN_DRIVER_NAME "ucan"
 #define UCAN_MAX_RX_URBS 8
 /* the CAN controller needs a while to enable/disable the bus */
-- 
2.19.1

^ permalink raw reply related

* [PATCH 06/20] dt-bindings: can: rcar_can: document r8a77965 support
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Eugeniu Rosca, Eugeniu Rosca,
	Marc Kleine-Budde
In-Reply-To: <20181112115728.18331-1-mkl@pengutronix.de>

From: Eugeniu Rosca <rosca.eugeniu@gmail.com>

Document the support for rcar_can on R8A77965 SoC devices.
Add R8A77965 to the list of SoCs which require the "assigned-clocks" and
"assigned-clock-rates" properties (thanks, Sergei).

Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt
index cc4372842bf3..47fc68148f38 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
@@ -14,6 +14,7 @@ Required properties:
 	      "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC.
 	      "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC.
 	      "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC.
+	      "renesas,can-r8a77965" if CAN controller is a part of R8A77965 SoC.
 	      "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device.
 	      "renesas,rcar-gen2-can" for a generic R-Car Gen2 or RZ/G1
 	      compatible device.
@@ -29,11 +30,10 @@ Required properties:
 - pinctrl-0: pin control group to be used for this controller.
 - pinctrl-names: must be "default".
 
-Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
-compatible:
-In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock
-and can be used by both CAN and CAN FD controller at the same time. It needs to
-be scaled to maximum frequency if any of these controllers use it. This is done
+Required properties for R8A7795, R8A7796 and R8A77965:
+For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock and can
+be used by both CAN and CAN FD controller at the same time. It needs to be
+scaled to maximum frequency if any of these controllers use it. This is done
 using the below properties:
 
 - assigned-clocks: phandle of clkp2(CANFD) clock.
-- 
2.19.1

^ permalink raw reply related

* [PATCH 07/20] can: rcar_can: Fix erroneous registration
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Fabrizio Castro, Chris Paterson,
	Marc Kleine-Budde
In-Reply-To: <20181112115728.18331-1-mkl@pengutronix.de>

From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

Assigning 2 to "renesas,can-clock-select" tricks the driver into
registering the CAN interface, even though we don't want that.
This patch improves one of the checks to prevent that from happening.

Fixes: 862e2b6af9413b43 ("can: rcar_can: support all input clocks")
Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_can.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/rcar/rcar_can.c b/drivers/net/can/rcar/rcar_can.c
index 11662f479e76..771a46083739 100644
--- a/drivers/net/can/rcar/rcar_can.c
+++ b/drivers/net/can/rcar/rcar_can.c
@@ -24,6 +24,9 @@
 
 #define RCAR_CAN_DRV_NAME	"rcar_can"
 
+#define RCAR_SUPPORTED_CLOCKS	(BIT(CLKR_CLKP1) | BIT(CLKR_CLKP2) | \
+				 BIT(CLKR_CLKEXT))
+
 /* Mailbox configuration:
  * mailbox 60 - 63 - Rx FIFO mailboxes
  * mailbox 56 - 59 - Tx FIFO mailboxes
@@ -789,7 +792,7 @@ static int rcar_can_probe(struct platform_device *pdev)
 		goto fail_clk;
 	}
 
-	if (clock_select >= ARRAY_SIZE(clock_names)) {
+	if (!(BIT(clock_select) & RCAR_SUPPORTED_CLOCKS)) {
 		err = -EINVAL;
 		dev_err(&pdev->dev, "invalid CAN clock selected\n");
 		goto fail_clk;
-- 
2.19.1

^ permalink raw reply related

* [PATCH 08/20] dt-bindings: can: rcar_can: Add r8a774a1 support
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Fabrizio Castro, Chris Paterson,
	Marc Kleine-Budde
In-Reply-To: <20181112115728.18331-1-mkl@pengutronix.de>

From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>

Document RZ/G2M (r8a774a1) SoC specific bindings.

Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>
Reviewed-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 .../devicetree/bindings/net/can/rcar_can.txt   | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt
index 47fc68148f38..9936b9ee67c3 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: "renesas,can-r8a7743" if CAN controller is a part of R8A7743 SoC.
 	      "renesas,can-r8a7744" if CAN controller is a part of R8A7744 SoC.
 	      "renesas,can-r8a7745" if CAN controller is a part of R8A7745 SoC.
+	      "renesas,can-r8a774a1" if CAN controller is a part of R8A774A1 SoC.
 	      "renesas,can-r8a7778" if CAN controller is a part of R8A7778 SoC.
 	      "renesas,can-r8a7779" if CAN controller is a part of R8A7779 SoC.
 	      "renesas,can-r8a7790" if CAN controller is a part of R8A7790 SoC.
@@ -18,15 +19,21 @@ Required properties:
 	      "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device.
 	      "renesas,rcar-gen2-can" for a generic R-Car Gen2 or RZ/G1
 	      compatible device.
-	      "renesas,rcar-gen3-can" for a generic R-Car Gen3 compatible device.
+	      "renesas,rcar-gen3-can" for a generic R-Car Gen3 or RZ/G2
+	      compatible device.
 	      When compatible with the generic version, nodes must list the
 	      SoC-specific version corresponding to the platform first
 	      followed by the generic version.
 
 - reg: physical base address and size of the R-Car CAN register map.
 - interrupts: interrupt specifier for the sole interrupt.
-- clocks: phandles and clock specifiers for 3 CAN clock inputs.
-- clock-names: 3 clock input name strings: "clkp1", "clkp2", "can_clk".
+- clocks: phandles and clock specifiers for 2 CAN clock inputs for RZ/G2
+	  devices.
+	  phandles and clock specifiers for 3 CAN clock inputs for every other
+	  SoC.
+- clock-names: 2 clock input name strings for RZ/G2: "clkp1", "can_clk".
+	       3 clock input name strings for every other SoC: "clkp1", "clkp2",
+	       "can_clk".
 - pinctrl-0: pin control group to be used for this controller.
 - pinctrl-names: must be "default".
 
@@ -42,8 +49,9 @@ using the below properties:
 Optional properties:
 - renesas,can-clock-select: R-Car CAN Clock Source Select. Valid values are:
 			    <0x0> (default) : Peripheral clock (clkp1)
-			    <0x1> : Peripheral clock (clkp2)
-			    <0x3> : Externally input clock
+			    <0x1> : Peripheral clock (clkp2) (not supported by
+				    RZ/G2 devices)
+			    <0x3> : External input clock
 
 Example
 -------
-- 
2.19.1

^ permalink raw reply related

* [PATCH 10/20] can: flexcan: Unlock the MB unconditionally
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Pankaj Bansal, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20181112115728.18331-1-mkl@pengutronix.de>

From: Pankaj Bansal <pankaj.bansal@nxp.com>

Unlock the MB irrespective of reception method being FIFO or timestamp
based. It is optional but recommended to unlock Mailbox as soon as
possible and make it available for reception.

Reported-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8e972ef08637..0431f8d05518 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -720,9 +720,14 @@ static unsigned int flexcan_mailbox_read(struct can_rx_offload *offload,
 			priv->write(BIT(n - 32), &regs->iflag2);
 	} else {
 		priv->write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
-		priv->read(&regs->timer);
 	}
 
+	/* Read the Free Running Timer. It is optional but recommended
+	 * to unlock Mailbox as soon as possible and make it available
+	 * for reception.
+	 */
+	priv->read(&regs->timer);
+
 	return 1;
 }
 
-- 
2.19.1

^ permalink raw reply related

* [PATCH 13/20] can: dev: can_get_echo_skb(): factor out non sending code to __can_get_echo_skb()
From: Marc Kleine-Budde @ 2018-11-12 11:57 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde, linux-stable
In-Reply-To: <20181112115728.18331-1-mkl@pengutronix.de>

This patch factors out all non sending parts of can_get_echo_skb() into
a seperate function __can_get_echo_skb(), so that it can be re-used in
an upcoming patch.

Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev.c   | 36 +++++++++++++++++++++++++-----------
 include/linux/can/dev.h |  1 +
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 49163570a63a..80530ab37b1e 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -477,14 +477,7 @@ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(can_put_echo_skb);
 
-/*
- * Get the skb from the stack and loop it back locally
- *
- * The function is typically called when the TX done interrupt
- * is handled in the device driver. The driver must protect
- * access to priv->echo_skb, if necessary.
- */
-unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
+struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr)
 {
 	struct can_priv *priv = netdev_priv(dev);
 
@@ -495,13 +488,34 @@ unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
 		struct can_frame *cf = (struct can_frame *)skb->data;
 		u8 dlc = cf->can_dlc;
 
-		netif_rx(priv->echo_skb[idx]);
+		*len_ptr = dlc;
 		priv->echo_skb[idx] = NULL;
 
-		return dlc;
+		return skb;
 	}
 
-	return 0;
+	return NULL;
+}
+
+/*
+ * Get the skb from the stack and loop it back locally
+ *
+ * The function is typically called when the TX done interrupt
+ * is handled in the device driver. The driver must protect
+ * access to priv->echo_skb, if necessary.
+ */
+unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx)
+{
+	struct sk_buff *skb;
+	u8 len;
+
+	skb = __can_get_echo_skb(dev, idx, &len);
+	if (!skb)
+		return 0;
+
+	netif_rx(skb);
+
+	return len;
 }
 EXPORT_SYMBOL_GPL(can_get_echo_skb);
 
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index a83e1f632eb7..f01623aef2f7 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -169,6 +169,7 @@ void can_change_state(struct net_device *dev, struct can_frame *cf,
 
 void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
 		      unsigned int idx);
+struct sk_buff *__can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr);
 unsigned int can_get_echo_skb(struct net_device *dev, unsigned int idx);
 void can_free_echo_skb(struct net_device *dev, unsigned int idx);
 
-- 
2.19.1

^ permalink raw reply related

* Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: Martin Blumenstingl @ 2018-11-12 21:53 UTC (permalink / raw)
  To: hkallweit1
  Cc: f.fainelli, andrew, netdev, richardcochran, linux-kernel,
	bcm-kernel-feedback-list, khilman, carlo, linux-amlogic, davem,
	linux-arm-kernel
In-Reply-To: <b6f0c1bb-9c0b-a02a-1571-b2804fcbcaa1@gmail.com>

On Mon, Nov 12, 2018 at 9:38 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 12.11.2018 21:32, Martin Blumenstingl wrote:
> > Hi Heiner,
> >
> > On Fri, Nov 9, 2018 at 9:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>
> >> On 09.11.2018 21:33, Florian Fainelli wrote:
> >>> On 11/9/18 12:22 PM, Heiner Kallweit wrote:
> >>>> On 09.11.2018 21:13, Andrew Lunn wrote:
> >>>>> Hi Heiner
> >>>>>
> >>>>>> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> >>>>>> +{
> >>>>>> +  return phydrv->config_intr || phydrv->ack_interrupt;
> >>>>>> +}
> >>>>>
> >>>>> Should this be && not || ? I thought both needed to be provided for
> >>>>> interrupts to work.
> >>>>>
> >>>>>        Andrew
> >>>>>
> >>>> I've seen at least one driver which configures interrupts in
> >>>> config_init and doesn't define a config_intr callback
> >>>> (ack_interrupt callback is there)
> >>>
> >>> That driver should probably be fixed, while it most likely does not make
> >>> any significant difference during probe/connect, since config_init() and
> >>> config_intr() are virtually happening at the same time, this is not
> >>> necessarily true when disconnecting from the PHY where we really want
> >>> config_intr() to effectively disable the interrupts and not leaving
> >>> something enabled that would now become unmaskable, because no more
> >>> driver attached.
> >>>
> >> Found the driver: It's the IP101A/G in icplus.c
> >> It should be easy to fix the behavior and move the interrupt config
> >> to a config_intr callback. But the last real changes to the driver
> >> have been done 6 years ago, so I'm not sure there's anybody out
> >> there who can test.
> > if you want I can take care of the IP101A/G code.
> > I have at least one board with an IP101A/G (PHY ID: 0x02430c54,
> > according to the schematics it's an IP101GR-GP) where the interrupt is
> > routed to the SoC.
> >
> > please let me know whether you'd like to work on it or if I should
> > give it a try.
> >
> I made the change already based on the datasheet of IP101A LF which
> is supposed to be register-compatible with IP101A/G.
> The patch is not applied yet, you can find it in the mailing
> list archive or in patchwork. Would be great if you could test it
> and report problems or add a Tested-by.
I will test it but I doubt it works out-of-the-box in my setup:
my board routes the RXER/INTR_32 pin of the IP101GR to the SoC. that
pin defaults to "RXER" (receive error signal). so I need to come up
with some extra patches which toggle the bit to output the interrupt
signal on that pin

in other words: I'll give your patches a try as soon as I have time
and give my Tested-by along with my extra patches on top of yours.


Regards
Martin

^ permalink raw reply

* [PATCH] bpf: Remove unused variable in nsim_bpf
From: Nathan Chancellor @ 2018-11-12 22:10 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Quentin Monnet, Jakub Kicinski, netdev, linux-kernel,
	Nathan Chancellor

Clang warns:

drivers/net/netdevsim/bpf.c:557:30: error: unused variable 'state'
[-Werror,-Wunused-variable]
        struct nsim_bpf_bound_prog *state;
                                    ^
1 error generated.

The declaration should have been removed in commit b07ade27e933 ("bpf:
pass translate() as a callback and remove its ndo_bpf subcommand").

Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/net/netdevsim/bpf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 6a5b7bd9a1f9..a1b29173ca1c 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -554,7 +554,6 @@ static void nsim_bpf_map_free(struct bpf_offloaded_map *offmap)
 int nsim_bpf(struct net_device *dev, struct netdev_bpf *bpf)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bpf_bound_prog *state;
 	int err;
 
 	ASSERT_RTNL();
-- 
2.19.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox