* Re: [PATCH for-next 05/10] IB/cm,cma: Move RDMA IP CM private-data parsing code from ib_cma to ib_cm
From: Yann Droneaud @ 2015-02-01 12:55 UTC (permalink / raw)
To: Shachar Raindel
Cc: roland, sean.hefty, linux-rdma, netdev, liranl, Guy Shapiro,
Haggai Eran, Yotam Kenneth
In-Reply-To: <1422790133-28725-6-git-send-email-raindel@mellanox.com>
Le dimanche 01 février 2015 à 13:28 +0200, Shachar Raindel a écrit :
> From: Guy Shapiro <guysh@mellanox.com>
>
> When receiving a connection request, ib_cm needs to associate the request with
> a network namespace. To do this, it needs to know the request's destination
> IP. For this the RDMA IP CM packet formatting functionality needs to be
> exposed to ib_cm.
>
> This patch merely moves the RDMA IP CM data formatting and parsing functions
> to be part of ib_cm. The following patch will utilize the new knowledge to
> look-up the appropriate namespace. Each namespace maintains an independent
> table of RDMA CM service IDs, allowing isolation and separation between the
> network namespaces.
>
> When creating a new incoming connection ID, the code in cm_save_ip_info can no
> longer rely on the listener's private data to find the port number, so it
> reads it from the requested service ID. This required saving the service ID in
> cm_format_paths_from_req.
>
> Signed-off-by: Guy Shapiro <guysh@mellanox.com>
> Signed-off-by: Haggai Eran <haggaie@mellanox.com>
> Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> Signed-off-by: Shachar Raindel <raindel@mellanox.com>
>
> ---
> drivers/infiniband/core/cm.c | 167 ++++++++++++++++++++++++++++++++++++++++++
> drivers/infiniband/core/cma.c | 166 +++++------------------------------------
> include/rdma/ib_cm.h | 46 ++++++++++++
> 3 files changed, 231 insertions(+), 148 deletions(-)
>
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> index 5a45cb76c43e..5cc1a4aa9728 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -51,6 +51,7 @@
>
> #include <rdma/ib_cache.h>
> #include <rdma/ib_cm.h>
> +#include <rdma/ib.h>
> #include "cm_msgs.h"
>
> MODULE_AUTHOR("Sean Hefty");
> @@ -701,6 +702,170 @@ static void cm_reject_sidr_req(struct cm_id_private *cm_id_priv,
> ib_send_cm_sidr_rep(&cm_id_priv->id, ¶m);
> }
>
> +static inline u8 cm_get_ip_ver(struct cm_hdr *hdr)
> +{
> + return hdr->ip_version >> 4;
> +}
> +
> +void cm_set_ip_ver(struct cm_hdr *hdr, u8 ip_ver)
> +{
> + hdr->ip_version = (ip_ver << 4) | (hdr->ip_version & 0xF);
> +}
> +EXPORT_SYMBOL(cm_set_ip_ver);
> +
That can be defined as an inline function in header.
> +int cm_format_hdr(void *hdr, int family,
> + struct sockaddr *src_addr,
> + struct sockaddr *dst_addr)
> +{
> + struct cm_hdr *cm_hdr;
> +
> + cm_hdr = hdr;
> + cm_hdr->cm_version = RDMA_IP_CM_VERSION;
> + if (family == AF_INET) {
> + struct sockaddr_in *src4, *dst4;
> +
> + src4 = (struct sockaddr_in *)src_addr;
> + dst4 = (struct sockaddr_in *)dst_addr;
> +
> + cm_set_ip_ver(cm_hdr, 4);
> + cm_hdr->src_addr.ip4.addr = src4->sin_addr.s_addr;
> + cm_hdr->dst_addr.ip4.addr = dst4->sin_addr.s_addr;
> + cm_hdr->port = src4->sin_port;
> + } else if (family == AF_INET6) {
> + struct sockaddr_in6 *src6, *dst6;
> +
> + src6 = (struct sockaddr_in6 *)src_addr;
> + dst6 = (struct sockaddr_in6 *)dst_addr;
> +
> + cm_set_ip_ver(cm_hdr, 6);
> + cm_hdr->src_addr.ip6 = src6->sin6_addr;
> + cm_hdr->dst_addr.ip6 = dst6->sin6_addr;
> + cm_hdr->port = src6->sin6_port;
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL(cm_format_hdr);
> +
> +static void cm_save_ib_info(struct sockaddr *src_addr,
> + struct sockaddr *dst_addr,
> + struct ib_sa_path_rec *path)
> +{
> + struct sockaddr_ib *ib;
> +
> + if (src_addr) {
> + ib = (struct sockaddr_ib *)src_addr;
> + ib->sib_family = AF_IB;
> + ib->sib_pkey = path->pkey;
> + ib->sib_flowinfo = path->flow_label;
> + memcpy(&ib->sib_addr, &path->sgid, 16);
> + ib->sib_sid = path->service_id;
> + ib->sib_sid_mask = cpu_to_be64(0xffffffffffffffffULL);
> + ib->sib_scope_id = 0;
> + }
> + if (dst_addr) {
> + ib = (struct sockaddr_ib *)dst_addr;
> + ib->sib_family = AF_IB;
> + ib->sib_pkey = path->pkey;
> + ib->sib_flowinfo = path->flow_label;
> + memcpy(&ib->sib_addr, &path->dgid, 16);
> + }
> +}
> +
> +static void cm_save_ip6_info(struct sockaddr *src_addr,
> + struct sockaddr *dst_addr,
> + struct cm_hdr *hdr,
> + __be16 local_port)
> +{
> + struct sockaddr_in6 *ip6;
> +
> + if (src_addr) {
> + ip6 = (struct sockaddr_in6 *)src_addr;
> + ip6->sin6_family = AF_INET6;
> + ip6->sin6_addr = hdr->dst_addr.ip6;
> + ip6->sin6_port = local_port;
> + }
> +
> + if (dst_addr) {
> + ip6 = (struct sockaddr_in6 *)dst_addr;
> + ip6->sin6_family = AF_INET6;
> + ip6->sin6_addr = hdr->src_addr.ip6;
> + ip6->sin6_port = hdr->port;
> + }
> +}
> +
> +static void cm_save_ip4_info(struct sockaddr *src_addr,
> + struct sockaddr *dst_addr,
> + struct cm_hdr *hdr,
> + __be16 local_port)
> +{
> + struct sockaddr_in *ip4;
> +
> + if (src_addr) {
> + ip4 = (struct sockaddr_in *)src_addr;
> + ip4->sin_family = AF_INET;
> + ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
> + ip4->sin_port = local_port;
> + }
> +
> + if (dst_addr) {
> + ip4 = (struct sockaddr_in *)dst_addr;
> + ip4->sin_family = AF_INET;
> + ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
> + ip4->sin_port = hdr->port;
> + }
> +}
> +
> +static __be16 cm_port_from_service_id(__be64 service_id)
> +{
> + return htons(be64_to_cpu(service_id));
> +}
> +
> +static int cm_save_ip_info(struct sockaddr *src_addr,
> + struct sockaddr *dst_addr,
> + struct cm_work *work)
> +{
> + struct cm_hdr *hdr;
> + __be16 port;
> +
> + hdr = work->cm_event.private_data;
> + if (hdr->cm_version != RDMA_IP_CM_VERSION)
> + return -EINVAL;
> +
> + port = cm_port_from_service_id(work->path->service_id);
> +
> + switch (cm_get_ip_ver(hdr)) {
> + case 4:
> + cm_save_ip4_info(src_addr, dst_addr, hdr, port);
> + break;
> + case 6:
> + cm_save_ip6_info(src_addr, dst_addr, hdr, port);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +int cm_save_net_info(struct sockaddr *src_addr,
> + struct sockaddr *dst_addr,
> + struct ib_cm_event *ib_event)
> +{
> + struct cm_work *work = container_of(ib_event, struct cm_work, cm_event);
> +
> + if ((rdma_port_get_link_layer(work->port->cm_dev->ib_device,
> + work->port->port_num) ==
> + IB_LINK_LAYER_INFINIBAND) &&
> + (ib_event->event == IB_CM_REQ_RECEIVED)) {
> + cm_save_ib_info(src_addr, dst_addr,
> + ib_event->param.req_rcvd.primary_path);
> + return 0;
> + }
> +
> + return cm_save_ip_info(src_addr, dst_addr, work);
> +}
> +EXPORT_SYMBOL(cm_save_net_info);
> +
> struct ib_cm_id *ib_create_cm_id(struct ib_device *device,
> ib_cm_handler cm_handler,
> void *context)
> @@ -1260,6 +1425,7 @@ static void cm_format_paths_from_req(struct cm_req_msg *req_msg,
> primary_path->packet_life_time =
> cm_req_get_primary_local_ack_timeout(req_msg);
> primary_path->packet_life_time -= (primary_path->packet_life_time > 0);
> + primary_path->service_id = req_msg->service_id;
>
> if (req_msg->alt_local_lid) {
> memset(alt_path, 0, sizeof *alt_path);
> @@ -1281,6 +1447,7 @@ static void cm_format_paths_from_req(struct cm_req_msg *req_msg,
> alt_path->packet_life_time =
> cm_req_get_alt_local_ack_timeout(req_msg);
> alt_path->packet_life_time -= (alt_path->packet_life_time > 0);
> + alt_path->service_id = req_msg->service_id;
> }
> }
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index aeb2417ec928..9f6faeb1de5f 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -179,23 +179,8 @@ struct iboe_mcast_work {
> struct cma_multicast *mc;
> };
>
> -union cma_ip_addr {
> - struct in6_addr ip6;
> - struct {
> - __be32 pad[3];
> - __be32 addr;
> - } ip4;
> -};
>
> -struct cma_hdr {
> - u8 cma_version;
> - u8 ip_version; /* IP version: 7:4 */
> - __be16 port;
> - union cma_ip_addr src_addr;
> - union cma_ip_addr dst_addr;
> -};
>
> -#define CMA_VERSION 0x00
>
> static int cma_comp(struct rdma_id_private *id_priv, enum rdma_cm_state comp)
> {
> @@ -234,16 +219,6 @@ static enum rdma_cm_state cma_exch(struct rdma_id_private *id_priv,
> return old;
> }
>
> -static inline u8 cma_get_ip_ver(struct cma_hdr *hdr)
> -{
> - return hdr->ip_version >> 4;
> -}
> -
> -static inline void cma_set_ip_ver(struct cma_hdr *hdr, u8 ip_ver)
> -{
> - hdr->ip_version = (ip_ver << 4) | (hdr->ip_version & 0xF);
> -}
> -
> static void cma_attach_to_dev(struct rdma_id_private *id_priv,
> struct cma_device *cma_dev)
> {
> @@ -839,93 +814,9 @@ static inline int cma_any_port(struct sockaddr *addr)
> return !cma_port(addr);
> }
>
> -static void cma_save_ib_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_id,
> - struct ib_sa_path_rec *path)
> -{
> - struct sockaddr_ib *listen_ib, *ib;
> -
> - listen_ib = (struct sockaddr_ib *) &listen_id->route.addr.src_addr;
> - ib = (struct sockaddr_ib *) &id->route.addr.src_addr;
> - ib->sib_family = listen_ib->sib_family;
> - ib->sib_pkey = path->pkey;
> - ib->sib_flowinfo = path->flow_label;
> - memcpy(&ib->sib_addr, &path->sgid, 16);
> - ib->sib_sid = listen_ib->sib_sid;
> - ib->sib_sid_mask = cpu_to_be64(0xffffffffffffffffULL);
> - ib->sib_scope_id = listen_ib->sib_scope_id;
> -
> - ib = (struct sockaddr_ib *) &id->route.addr.dst_addr;
> - ib->sib_family = listen_ib->sib_family;
> - ib->sib_pkey = path->pkey;
> - ib->sib_flowinfo = path->flow_label;
> - memcpy(&ib->sib_addr, &path->dgid, 16);
> -}
> -
> -static void cma_save_ip4_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_id,
> - struct cma_hdr *hdr)
> -{
> - struct sockaddr_in *listen4, *ip4;
> -
> - listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
> - ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
> - ip4->sin_family = AF_INET;
> - ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
> - ip4->sin_port = listen4->sin_port;
> -
> - ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr;
> - ip4->sin_family = AF_INET;
> - ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
> - ip4->sin_port = hdr->port;
> -}
> -
> -static void cma_save_ip6_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_id,
> - struct cma_hdr *hdr)
> -{
> - struct sockaddr_in6 *listen6, *ip6;
> -
> - listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr;
> - ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr;
> - ip6->sin6_family = AF_INET6;
> - ip6->sin6_addr = hdr->dst_addr.ip6;
> - ip6->sin6_port = listen6->sin6_port;
> -
> - ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr;
> - ip6->sin6_family = AF_INET6;
> - ip6->sin6_addr = hdr->src_addr.ip6;
> - ip6->sin6_port = hdr->port;
> -}
> -
> -static int cma_save_net_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_id,
> - struct ib_cm_event *ib_event)
> -{
> - struct cma_hdr *hdr;
> -
> - if ((listen_id->route.addr.src_addr.ss_family == AF_IB) &&
> - (ib_event->event == IB_CM_REQ_RECEIVED)) {
> - cma_save_ib_info(id, listen_id, ib_event->param.req_rcvd.primary_path);
> - return 0;
> - }
> -
> - hdr = ib_event->private_data;
> - if (hdr->cma_version != CMA_VERSION)
> - return -EINVAL;
> -
> - switch (cma_get_ip_ver(hdr)) {
> - case 4:
> - cma_save_ip4_info(id, listen_id, hdr);
> - break;
> - case 6:
> - cma_save_ip6_info(id, listen_id, hdr);
> - break;
> - default:
> - return -EINVAL;
> - }
> - return 0;
> -}
> -
> static inline int cma_user_data_offset(struct rdma_id_private *id_priv)
> {
> - return cma_family(id_priv) == AF_IB ? 0 : sizeof(struct cma_hdr);
> + return cma_family(id_priv) == AF_IB ? 0 : sizeof(struct cm_hdr);
> }
>
> static void cma_cancel_route(struct rdma_id_private *id_priv)
> @@ -1195,7 +1086,9 @@ static struct rdma_id_private *cma_new_conn_id(struct rdma_cm_id *listen_id,
> return NULL;
>
> id_priv = container_of(id, struct rdma_id_private, id);
> - if (cma_save_net_info(id, listen_id, ib_event))
> + if (cm_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
> + (struct sockaddr *)&id->route.addr.dst_addr,
> + ib_event))
> goto err;
>
> rt = &id->route;
> @@ -1241,7 +1134,9 @@ static struct rdma_id_private *cma_new_udp_id(struct rdma_cm_id *listen_id,
> return NULL;
>
> id_priv = container_of(id, struct rdma_id_private, id);
> - if (cma_save_net_info(id, listen_id, ib_event))
> + if (cm_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
> + (struct sockaddr *)&id->route.addr.dst_addr,
> + ib_event))
> goto err;
>
> if (!cma_any_addr((struct sockaddr *) &id->route.addr.src_addr)) {
> @@ -1369,7 +1264,7 @@ EXPORT_SYMBOL(rdma_get_service_id);
> static void cma_set_compare_data(enum rdma_port_space ps, struct sockaddr *addr,
> struct ib_cm_compare_data *compare)
> {
> - struct cma_hdr *cma_data, *cma_mask;
> + struct cm_hdr *cma_data, *cma_mask;
> __be32 ip4_addr;
> struct in6_addr ip6_addr;
>
> @@ -1380,8 +1275,8 @@ static void cma_set_compare_data(enum rdma_port_space ps, struct sockaddr *addr,
> switch (addr->sa_family) {
> case AF_INET:
> ip4_addr = ((struct sockaddr_in *) addr)->sin_addr.s_addr;
> - cma_set_ip_ver(cma_data, 4);
> - cma_set_ip_ver(cma_mask, 0xF);
> + cm_set_ip_ver(cma_data, 4);
> + cm_set_ip_ver(cma_mask, 0xF);
> if (!cma_any_addr(addr)) {
> cma_data->dst_addr.ip4.addr = ip4_addr;
> cma_mask->dst_addr.ip4.addr = htonl(~0);
> @@ -1389,8 +1284,8 @@ static void cma_set_compare_data(enum rdma_port_space ps, struct sockaddr *addr,
> break;
> case AF_INET6:
> ip6_addr = ((struct sockaddr_in6 *) addr)->sin6_addr;
> - cma_set_ip_ver(cma_data, 6);
> - cma_set_ip_ver(cma_mask, 0xF);
> + cm_set_ip_ver(cma_data, 6);
> + cm_set_ip_ver(cma_mask, 0xF);
> if (!cma_any_addr(addr)) {
> cma_data->dst_addr.ip6 = ip6_addr;
> memset(&cma_mask->dst_addr.ip6, 0xFF,
> @@ -2615,35 +2510,6 @@ err1:
> }
> EXPORT_SYMBOL(rdma_bind_addr);
>
> -static int cma_format_hdr(void *hdr, struct rdma_id_private *id_priv)
> -{
> - struct cma_hdr *cma_hdr;
> -
> - cma_hdr = hdr;
> - cma_hdr->cma_version = CMA_VERSION;
> - if (cma_family(id_priv) == AF_INET) {
> - struct sockaddr_in *src4, *dst4;
> -
> - src4 = (struct sockaddr_in *) cma_src_addr(id_priv);
> - dst4 = (struct sockaddr_in *) cma_dst_addr(id_priv);
> -
> - cma_set_ip_ver(cma_hdr, 4);
> - cma_hdr->src_addr.ip4.addr = src4->sin_addr.s_addr;
> - cma_hdr->dst_addr.ip4.addr = dst4->sin_addr.s_addr;
> - cma_hdr->port = src4->sin_port;
> - } else if (cma_family(id_priv) == AF_INET6) {
> - struct sockaddr_in6 *src6, *dst6;
> -
> - src6 = (struct sockaddr_in6 *) cma_src_addr(id_priv);
> - dst6 = (struct sockaddr_in6 *) cma_dst_addr(id_priv);
> -
> - cma_set_ip_ver(cma_hdr, 6);
> - cma_hdr->src_addr.ip6 = src6->sin6_addr;
> - cma_hdr->dst_addr.ip6 = dst6->sin6_addr;
> - cma_hdr->port = src6->sin6_port;
> - }
> - return 0;
> -}
>
> static int cma_sidr_rep_handler(struct ib_cm_id *cm_id,
> struct ib_cm_event *ib_event)
> @@ -2731,7 +2597,9 @@ static int cma_resolve_ib_udp(struct rdma_id_private *id_priv,
> conn_param->private_data_len);
>
> if (private_data) {
> - ret = cma_format_hdr(private_data, id_priv);
> + ret = cm_format_hdr(private_data, cma_family(id_priv),
> + cma_src_addr(id_priv),
> + cma_dst_addr(id_priv));
> if (ret)
> goto out;
> req.private_data = private_data;
> @@ -2796,7 +2664,9 @@ static int cma_connect_ib(struct rdma_id_private *id_priv,
>
> route = &id_priv->id.route;
> if (private_data) {
> - ret = cma_format_hdr(private_data, id_priv);
> + ret = cm_format_hdr(private_data, cma_family(id_priv),
> + cma_src_addr(id_priv),
> + cma_dst_addr(id_priv));
> if (ret)
> goto out;
> req.private_data = private_data;
> diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
> index 0e3ff30647d5..e418a11afcfe 100644
> --- a/include/rdma/ib_cm.h
> +++ b/include/rdma/ib_cm.h
> @@ -274,6 +274,52 @@ struct ib_cm_event {
> #define CM_LAP_ATTR_ID cpu_to_be16(0x0019)
> #define CM_APR_ATTR_ID cpu_to_be16(0x001A)
>
> +union cm_ip_addr {
> + struct in6_addr ip6;
> + struct {
> + __be32 pad[3];
> + __be32 addr;
> + } ip4;
> +};
> +
> +struct cm_hdr {
> + u8 cm_version;
> + u8 ip_version; /* IP version: 7:4 */
> + __be16 port;
> + union cm_ip_addr src_addr;
> + union cm_ip_addr dst_addr;
> +};
> +
> +#define RDMA_IP_CM_VERSION 0x00
> +
> +/**
> + * cm_format_hdr - Fill in a cm_hdr struct according to connection details
> + * @hdr: cm_hdr struct to fill
> + * @family: ip family of the addresses - AF_INET or AF_INTET6
> + * @src_addr: source address of the connection
> + * @dst_addr: destination address of the connection
> + **/
> +int cm_format_hdr(void *hdr, int family,
> + struct sockaddr *src_addr,
> + struct sockaddr *dst_addr);
> +
> +/**
> + * cm_save_net_info - saves ib connection event details
> + * @src_addr: source address of the connection
> + * @dst_addr: destination address of the connection
> + * @ib_event: ib event to take connection details from
> + **/
> +int cm_save_net_info(struct sockaddr *src_addr,
> + struct sockaddr *dst_addr,
> + struct ib_cm_event *ib_event);
> +
> +/**
> + * cm_set_ip_ver - sets the ip version of a cm_hdr struct
> + * @hdr: cm_hdr struct to change
> + * @ip_ver: ip version to set - a 4 bit value
> + **/
> +void cm_set_ip_ver(struct cm_hdr *hdr, u8 ip_ver);
> +
> /**
> * ib_cm_handler - User-defined callback to process communication events.
> * @cm_id: Communication identifier associated with the reported event.
Every other symbols in ib_cm.h are prefixed by "ib_cm_", so I would
prefer having symbols moved from ib_cma.c to ib_cm.c be renamed, except
it would create a lot of code change ...
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply
* Re: [PATCH] tun: orphan an skb on tx
From: David Woodhouse @ 2015-02-01 13:33 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Herbert Xu, Eric Dumazet, Jan Kiszka, David S. Miller, Paul Moore,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org, qemu-devel
In-Reply-To: <20150201122611.GA8883@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]
On Sun, 2015-02-01 at 14:26 +0200, Michael S. Tsirkin wrote:
> > When I run over the VPN, netperf thinks it sent 2½ times the amount of
> > TX traffic.
>
> At some level, it's expected: netperf's manual actually says:
> A UDP_STREAM test has no end-to-end flow control - UDP provides none and
> neither does netperf. However, if you wish, you can configure netperf
> with --enable-intervals=yes to enable the global command-line -b and -w
> options to pace bursts of traffic onto the network.
True, but UDP is just a canary for other protocols here. We *should* be
able to keep track of the packets before they even leave the box, and
know that we haven't even managed to send them. Even if we know it's a
datagram protocol and it's potentially going to be dropped in transit
later.
Of course, now I'm looking closely at the path these packets take to
leave the box, it starts to offend me that they're being passed up to
userspace just to encrypt them (as DTLS or ESP) and then send them back
down to the kernel on a UDP socket. The kernel already knows how to
{en,de}crypt ESP, and do the sequence number checking on incoming
packets.
I'm wondering if we bypass userspace in that case somehow — let
userspace negotiate the encryption and connect the UDP socket, then just
pass the socket fd and the parameters to the kernel so that incoming
packets are decrypted and 'received' on the tun device, and outgoing
packets on the tun device are encrypted and sent out on the UDP socket.
The performance isn't too much of an issue for a VPN *client* in
practice, but we have a server implementation too which would probably
benefit quite well from such an offload facility.
If I were to look at such a thing, would it provoke screams of horror?
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]
^ permalink raw reply
* Re: [PATCH for-next 08/10] IB/cma: Add support for network namespaces
From: Yann Droneaud @ 2015-02-01 13:44 UTC (permalink / raw)
To: Shachar Raindel
Cc: roland, sean.hefty, linux-rdma, netdev, liranl, Guy Shapiro,
Haggai Eran, Yotam Kenneth
In-Reply-To: <1422790133-28725-9-git-send-email-raindel@mellanox.com>
Hi,
Le dimanche 01 février 2015 à 13:28 +0200, Shachar Raindel a écrit :
> From: Guy Shapiro <guysh@mellanox.com>
>
> Add support for network namespaces in the ib_cma module. This is
> accomplished by:
>
> 1. Adding network namespace parameter for rdma_create_id. This parameter is used
> to populate the network namespace field in rdma_id_private. rdma_create_id
> keeps a reference on the network namespace.
> 2. Using the network namespace from the rdma_id instead of init_net inside of
> ib_cma.
> 3. Decrementing the reference count for the appropriate network namespace when
> calling rdma_destroy_id.
>
> In order to preserve the current behavior init_net is passed when calling from
> other modules.
>
> Signed-off-by: Guy Shapiro <guysh@mellanox.com>
> Signed-off-by: Haggai Eran <haggaie@mellanox.com>
> Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> Signed-off-by: Shachar Raindel <raindel@mellanox.com>
>
> ---
> drivers/infiniband/core/cma.c | 52 +++++++++++++---------
> drivers/infiniband/core/ucma.c | 3 +-
> drivers/infiniband/ulp/iser/iser_verbs.c | 2 +-
> drivers/infiniband/ulp/isert/ib_isert.c | 2 +-
> .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h | 4 +-
> include/rdma/rdma_cm.h | 6 ++-
> net/9p/trans_rdma.c | 2 +-
> net/rds/ib.c | 2 +-
> net/rds/ib_cm.c | 2 +-
> net/rds/iw.c | 2 +-
> net/rds/iw_cm.c | 2 +-
> net/rds/rdma_transport.c | 2 +-
> net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +-
> net/sunrpc/xprtrdma/verbs.c | 3 +-
> 14 files changed, 52 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 022b0d0a51cc..f6379b38b366 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -540,7 +540,8 @@ static int cma_disable_callback(struct rdma_id_private *id_priv,
>
> struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
> void *context, enum rdma_port_space ps,
> - enum ib_qp_type qp_type)
> + enum ib_qp_type qp_type,
> + struct net *net)
> {
> struct rdma_id_private *id_priv;
>
> @@ -562,7 +563,7 @@ struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
> INIT_LIST_HEAD(&id_priv->listen_list);
> INIT_LIST_HEAD(&id_priv->mc_list);
> get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
> - id_priv->id.route.addr.dev_addr.net = &init_net;
> + id_priv->id.route.addr.dev_addr.net = get_net(net);
>
> return &id_priv->id;
> }
> @@ -689,7 +690,7 @@ static int cma_modify_qp_rtr(struct rdma_id_private *id_priv,
> rdma_port_get_link_layer(id_priv->id.device, id_priv->id.port_num)
> == IB_LINK_LAYER_ETHERNET) {
> ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL,
> - &init_net);
> + id_priv->id.route.addr.dev_addr.net);
>
> if (ret)
> goto out;
> @@ -953,6 +954,7 @@ static void cma_cancel_operation(struct rdma_id_private *id_priv,
> static void cma_release_port(struct rdma_id_private *id_priv)
> {
> struct rdma_bind_list *bind_list = id_priv->bind_list;
> + struct net *net = id_priv->id.route.addr.dev_addr.net;
>
> if (!bind_list)
> return;
> @@ -960,7 +962,7 @@ static void cma_release_port(struct rdma_id_private *id_priv)
> mutex_lock(&lock);
> hlist_del(&id_priv->node);
> if (hlist_empty(&bind_list->owners)) {
> - cma_ps_remove(bind_list->ps, &init_net, bind_list->port);
> + cma_ps_remove(bind_list->ps, net, bind_list->port);
> kfree(bind_list);
> }
> mutex_unlock(&lock);
> @@ -1029,6 +1031,7 @@ void rdma_destroy_id(struct rdma_cm_id *id)
> cma_deref_id(id_priv->id.context);
>
> kfree(id_priv->id.route.path_rec);
> + put_net(id_priv->id.route.addr.dev_addr.net);
> kfree(id_priv);
> }
> EXPORT_SYMBOL(rdma_destroy_id);
> @@ -1156,7 +1159,8 @@ static struct rdma_id_private *cma_new_conn_id(struct rdma_cm_id *listen_id,
> int ret;
>
> id = rdma_create_id(listen_id->event_handler, listen_id->context,
> - listen_id->ps, ib_event->param.req_rcvd.qp_type);
> + listen_id->ps, ib_event->param.req_rcvd.qp_type,
> + listen_id->route.addr.dev_addr.net);
> if (IS_ERR(id))
> return NULL;
>
> @@ -1201,10 +1205,11 @@ static struct rdma_id_private *cma_new_udp_id(struct rdma_cm_id *listen_id,
> {
> struct rdma_id_private *id_priv;
> struct rdma_cm_id *id;
> + struct net *net = listen_id->route.addr.dev_addr.net;
> int ret;
>
> id = rdma_create_id(listen_id->event_handler, listen_id->context,
> - listen_id->ps, IB_QPT_UD);
> + listen_id->ps, IB_QPT_UD, net);
> if (IS_ERR(id))
> return NULL;
>
> @@ -1455,7 +1460,8 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id,
> /* Create a new RDMA id for the new IW CM ID */
> new_cm_id = rdma_create_id(listen_id->id.event_handler,
> listen_id->id.context,
> - RDMA_PS_TCP, IB_QPT_RC);
> + RDMA_PS_TCP, IB_QPT_RC,
> + listen_id->id.route.addr.dev_addr.net);
> if (IS_ERR(new_cm_id)) {
> ret = -ENOMEM;
> goto out;
> @@ -1528,11 +1534,11 @@ static int cma_ib_listen(struct rdma_id_private *id_priv)
> struct ib_cm_compare_data compare_data;
> struct sockaddr *addr;
> struct ib_cm_id *id;
> + struct net *net = id_priv->id.route.addr.dev_addr.net;
> __be64 svc_id;
> int ret;
>
> - id = ib_create_cm_id(id_priv->id.device, cma_req_handler, id_priv,
> - &init_net);
> + id = ib_create_cm_id(id_priv->id.device, cma_req_handler, id_priv, net);
> if (IS_ERR(id))
> return PTR_ERR(id);
>
> @@ -1596,6 +1602,7 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> {
> struct rdma_id_private *dev_id_priv;
> struct rdma_cm_id *id;
> + struct net *net = id_priv->id.route.addr.dev_addr.net;
> int ret;
>
> if (cma_family(id_priv) == AF_IB &&
> @@ -1603,7 +1610,7 @@ static void cma_listen_on_dev(struct rdma_id_private *id_priv,
> return;
>
> id = rdma_create_id(cma_listen_handler, id_priv, id_priv->id.ps,
> - id_priv->id.qp_type);
> + id_priv->id.qp_type, net);
> if (IS_ERR(id))
> return;
>
> @@ -2283,7 +2290,8 @@ static int cma_alloc_port(struct radix_tree_root *ps,
> if (!bind_list)
> return -ENOMEM;
>
> - ret = cma_ps_alloc(ps, &init_net, bind_list, snum);
> + ret = cma_ps_alloc(ps, id_priv->id.route.addr.dev_addr.net, bind_list,
> + snum);
> if (ret < 0)
> goto err;
>
> @@ -2302,13 +2310,14 @@ static int cma_alloc_any_port(struct radix_tree_root *ps,
> static unsigned int last_used_port;
> int low, high, remaining;
> unsigned int rover;
> + struct net *net = id_priv->id.route.addr.dev_addr.net;
>
> - inet_get_local_port_range(&init_net, &low, &high);
> + inet_get_local_port_range(net, &low, &high);
> remaining = (high - low) + 1;
> rover = prandom_u32() % remaining + low;
> retry:
> if (last_used_port != rover &&
> - !cma_ps_find(ps, &init_net, (unsigned short)rover)) {
> + !cma_ps_find(ps, net, (unsigned short)rover)) {
> int ret = cma_alloc_port(ps, id_priv, rover);
> /*
> * Remember previously used port number in order to avoid
> @@ -2376,7 +2385,7 @@ static int cma_use_port(struct radix_tree_root *ps,
> if (snum < PROT_SOCK && !capable(CAP_NET_BIND_SERVICE))
> return -EACCES;
>
> - bind_list = cma_ps_find(ps, &init_net, snum);
> + bind_list = cma_ps_find(ps, id_priv->id.route.addr.dev_addr.net, snum);
> if (!bind_list) {
> ret = cma_alloc_port(ps, id_priv, snum);
> } else {
> @@ -2573,8 +2582,11 @@ int rdma_bind_addr(struct rdma_cm_id *id, struct sockaddr *addr)
> if (addr->sa_family == AF_INET)
> id_priv->afonly = 1;
> #if IS_ENABLED(CONFIG_IPV6)
> - else if (addr->sa_family == AF_INET6)
> - id_priv->afonly = init_net.ipv6.sysctl.bindv6only;
> + else if (addr->sa_family == AF_INET6) {
> + struct net *net = id_priv->id.route.addr.dev_addr.net;
> +
> + id_priv->afonly = net->ipv6.sysctl.bindv6only;
> + }
> #endif
> }
> ret = cma_get_port(id_priv);
> @@ -2687,7 +2699,7 @@ static int cma_resolve_ib_udp(struct rdma_id_private *id_priv,
> }
>
> id = ib_create_cm_id(id_priv->id.device, cma_sidr_rep_handler,
> - id_priv, &init_net);
> + id_priv, id_priv->id.route.addr.dev_addr.net);
> if (IS_ERR(id)) {
> ret = PTR_ERR(id);
> goto out;
> @@ -2737,7 +2749,7 @@ static int cma_connect_ib(struct rdma_id_private *id_priv,
> conn_param->private_data_len);
>
> id = ib_create_cm_id(id_priv->id.device, cma_ib_handler, id_priv,
> - &init_net);
> + id_priv->id.route.addr.dev_addr.net);
> if (IS_ERR(id)) {
> ret = PTR_ERR(id);
> goto out;
> @@ -3387,6 +3399,7 @@ static int cma_netdev_change(struct net_device *ndev, struct rdma_id_private *id
> dev_addr = &id_priv->id.route.addr.dev_addr;
>
> if ((dev_addr->bound_dev_if == ndev->ifindex) &&
> + (dev_net(ndev) == dev_addr->net) &&
net_eq() ?
> memcmp(dev_addr->src_dev_addr, ndev->dev_addr, ndev->addr_len)) {
> printk(KERN_INFO "RDMA CM addr change for ndev %s used by id %p\n",
> ndev->name, &id_priv->id);
> @@ -3412,9 +3425,6 @@ static int cma_netdev_callback(struct notifier_block *self, unsigned long event,
> struct rdma_id_private *id_priv;
> int ret = NOTIFY_DONE;
>
> - if (dev_net(ndev) != &init_net)
> - return NOTIFY_DONE;
> -
> if (event != NETDEV_BONDING_FAILOVER)
> return NOTIFY_DONE;
>
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 56a4b7ca7ee3..de755f2c6166 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -391,7 +391,8 @@ static ssize_t ucma_create_id(struct ucma_file *file, const char __user *inbuf,
> return -ENOMEM;
>
> ctx->uid = cmd.uid;
> - ctx->cm_id = rdma_create_id(ucma_event_handler, ctx, cmd.ps, qp_type);
> + ctx->cm_id = rdma_create_id(ucma_event_handler, ctx, cmd.ps, qp_type,
> + &init_net);
> if (IS_ERR(ctx->cm_id)) {
> ret = PTR_ERR(ctx->cm_id);
> goto err1;
> diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
> index 695a2704bd43..d4e9c639ad2f 100644
> --- a/drivers/infiniband/ulp/iser/iser_verbs.c
> +++ b/drivers/infiniband/ulp/iser/iser_verbs.c
> @@ -949,7 +949,7 @@ int iser_connect(struct iser_conn *iser_conn,
>
> ib_conn->cma_id = rdma_create_id(iser_cma_handler,
> (void *)iser_conn,
> - RDMA_PS_TCP, IB_QPT_RC);
> + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> if (IS_ERR(ib_conn->cma_id)) {
> err = PTR_ERR(ib_conn->cma_id);
> iser_err("rdma_create_id failed: %d\n", err);
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index dafb3c531f96..44a6fff8dc79 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> @@ -2960,7 +2960,7 @@ isert_setup_id(struct isert_np *isert_np)
> isert_dbg("ksockaddr: %p, sa: %p\n", &np->np_sockaddr, sa);
>
> id = rdma_create_id(isert_cma_handler, isert_np,
> - RDMA_PS_TCP, IB_QPT_RC);
> + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> if (IS_ERR(id)) {
> isert_err("rdma_create_id() failed: %ld\n", PTR_ERR(id));
> ret = PTR_ERR(id);
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
> index b02b4ec1e29d..128de4eb0959 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
> @@ -125,7 +125,9 @@ extern kib_tunables_t kiblnd_tunables;
> IBLND_CREDIT_HIGHWATER_V1 : \
> *kiblnd_tunables.kib_peercredits_hiw) /* when eagerly to return credits */
>
> -#define kiblnd_rdma_create_id(cb, dev, ps, qpt) rdma_create_id(cb, dev, ps, qpt)
> +#define kiblnd_rdma_create_id(cb, dev, ps, qpt) rdma_create_id(cb, dev, \
> + ps, qpt, \
> + &init_net)
>
> static inline int
> kiblnd_concurrent_sends_v1(void)
> diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> index 1ed2088dc9f5..3953e9c8bc94 100644
> --- a/include/rdma/rdma_cm.h
> +++ b/include/rdma/rdma_cm.h
> @@ -163,10 +163,14 @@ struct rdma_cm_id {
> * @context: User specified context associated with the id.
> * @ps: RDMA port space.
> * @qp_type: type of queue pair associated with the id.
> + * @net: The network namespace in which to create the new id.
> + *
> + * The id holds a reference on the network namespace until it is destroyed.
> */
> struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
> void *context, enum rdma_port_space ps,
> - enum ib_qp_type qp_type);
> + enum ib_qp_type qp_type,
> + struct net *net);
>
> /**
> * rdma_destroy_id - Destroys an RDMA identifier.
> diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
> index 14ad43b5cf89..577fd3129bcf 100644
> --- a/net/9p/trans_rdma.c
> +++ b/net/9p/trans_rdma.c
> @@ -635,7 +635,7 @@ rdma_create_trans(struct p9_client *client, const char *addr, char *args)
>
> /* Create the RDMA CM ID */
> rdma->cm_id = rdma_create_id(p9_cm_event_handler, client, RDMA_PS_TCP,
> - IB_QPT_RC);
> + IB_QPT_RC, &init_net);
> if (IS_ERR(rdma->cm_id))
> goto error;
>
> diff --git a/net/rds/ib.c b/net/rds/ib.c
> index ba2dffeff608..cc137f523248 100644
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -326,7 +326,7 @@ static int rds_ib_laddr_check(__be32 addr)
> /* Create a CMA ID and try to bind it. This catches both
> * IB and iWARP capable NICs.
> */
> - cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC);
> + cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC, &init_net);
> if (IS_ERR(cm_id))
> return PTR_ERR(cm_id);
>
> diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
> index 31b74f5e61ad..d19b91296ddc 100644
> --- a/net/rds/ib_cm.c
> +++ b/net/rds/ib_cm.c
> @@ -584,7 +584,7 @@ int rds_ib_conn_connect(struct rds_connection *conn)
> /* XXX I wonder what affect the port space has */
> /* delegate cm event handler to rdma_transport */
> ic->i_cm_id = rdma_create_id(rds_rdma_cm_event_handler, conn,
> - RDMA_PS_TCP, IB_QPT_RC);
> + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> if (IS_ERR(ic->i_cm_id)) {
> ret = PTR_ERR(ic->i_cm_id);
> ic->i_cm_id = NULL;
> diff --git a/net/rds/iw.c b/net/rds/iw.c
> index 589935661d66..8501b73ed12f 100644
> --- a/net/rds/iw.c
> +++ b/net/rds/iw.c
> @@ -227,7 +227,7 @@ static int rds_iw_laddr_check(__be32 addr)
> /* Create a CMA ID and try to bind it. This catches both
> * IB and iWARP capable NICs.
> */
> - cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC);
> + cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC, &init_net);
> if (IS_ERR(cm_id))
> return PTR_ERR(cm_id);
>
> diff --git a/net/rds/iw_cm.c b/net/rds/iw_cm.c
> index a91e1db62ee6..e5ee2d562a60 100644
> --- a/net/rds/iw_cm.c
> +++ b/net/rds/iw_cm.c
> @@ -521,7 +521,7 @@ int rds_iw_conn_connect(struct rds_connection *conn)
> /* XXX I wonder what affect the port space has */
> /* delegate cm event handler to rdma_transport */
> ic->i_cm_id = rdma_create_id(rds_rdma_cm_event_handler, conn,
> - RDMA_PS_TCP, IB_QPT_RC);
> + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> if (IS_ERR(ic->i_cm_id)) {
> ret = PTR_ERR(ic->i_cm_id);
> ic->i_cm_id = NULL;
> diff --git a/net/rds/rdma_transport.c b/net/rds/rdma_transport.c
> index 6cd9d1deafc3..066b60b27b12 100644
> --- a/net/rds/rdma_transport.c
> +++ b/net/rds/rdma_transport.c
> @@ -160,7 +160,7 @@ static int rds_rdma_listen_init(void)
> int ret;
>
> cm_id = rdma_create_id(rds_rdma_cm_event_handler, NULL, RDMA_PS_TCP,
> - IB_QPT_RC);
> + IB_QPT_RC, &init_net);
> if (IS_ERR(cm_id)) {
> ret = PTR_ERR(cm_id);
> printk(KERN_ERR "RDS/RDMA: failed to setup listener, "
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> index 4e618808bc98..e3b246e305f9 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> @@ -701,7 +701,7 @@ static struct svc_xprt *svc_rdma_create(struct svc_serv *serv,
> xprt = &cma_xprt->sc_xprt;
>
> listen_id = rdma_create_id(rdma_listen_handler, cma_xprt, RDMA_PS_TCP,
> - IB_QPT_RC);
> + IB_QPT_RC, &init_net);
> if (IS_ERR(listen_id)) {
> ret = PTR_ERR(listen_id);
> dprintk("svcrdma: rdma_create_id failed = %d\n", ret);
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index c98e40643910..f574e77165f4 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -483,7 +483,8 @@ rpcrdma_create_id(struct rpcrdma_xprt *xprt,
>
> init_completion(&ia->ri_done);
>
> - id = rdma_create_id(rpcrdma_conn_upcall, xprt, RDMA_PS_TCP, IB_QPT_RC);
> + id = rdma_create_id(rpcrdma_conn_upcall, xprt, RDMA_PS_TCP, IB_QPT_RC,
> + &init_net);
> if (IS_ERR(id)) {
> rc = PTR_ERR(id);
> dprintk("RPC: %s: rdma_create_id() failed %i\n",
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply
* RE: [PATCH for-next 01/10] IB/addr: Pass network namespace as a parameter
From: Shachar Raindel @ 2015-02-01 13:46 UTC (permalink / raw)
To: Yann Droneaud
Cc: roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Liran Liss,
Guy Shapiro, Haggai Eran, Yotam Kenneth
In-Reply-To: <1422793376.3030.37.camel-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
> -----Original Message-----
> From: Yann Droneaud [mailto:ydroneaud@opteya.com]
> Sent: Sunday, February 01, 2015 2:23 PM
> To: Shachar Raindel
> Cc: roland@kernel.org; sean.hefty@intel.com; linux-rdma@vger.kernel.org;
> netdev@vger.kernel.org; Liran Liss; Guy Shapiro; Haggai Eran; Yotam
> Kenneth
> Subject: Re: [PATCH for-next 01/10] IB/addr: Pass network namespace as a
> parameter
>
> Hi,
>
> Le dimanche 01 février 2015 à 13:28 +0200, Shachar Raindel a écrit :
> > From: Guy Shapiro <guysh@mellanox.com>
> >
> > Add network namespace support to the ib_addr module. For that, all the
> address
> > resolution and matching should be done using the appropriate namespace
> instead
> > of init_net.
> >
> > This is achieved by:
> >
> > 1. Adding an explicit network namespace argument to exported function
> that
> > require a namespace.
> > 2. Saving the namespace in the rdma_addr_client structure.
> > 3. Using it when calling networking functions.
> >
> > In order to preserve the behavior of calling modules, &init_net is
> > passed as the parameter in calls from other modules. This is modified
> as
> > namspace support is added on more levels.
>
> typo: "namespace"
>
Thanks. Will fix in next iteration.
> >
> > Signed-off-by: Haggai Eran <haggaie@mellanox.com>
> > Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> > Signed-off-by: Shachar Raindel <raindel@mellanox.com>
> > Signed-off-by: Guy Shapiro <guysh@mellanox.com>
> >
> > ---
> > drivers/infiniband/core/addr.c | 31 ++++++++++++----------
> > drivers/infiniband/core/cma.c | 4 ++-
> > drivers/infiniband/core/verbs.c | 14 +++++++---
> > drivers/infiniband/hw/ocrdma/ocrdma_ah.c | 3 ++-
> > include/rdma/ib_addr.h | 44
> ++++++++++++++++++++++++++++----
> > 5 files changed, 72 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/addr.c
> b/drivers/infiniband/core/addr.c
> > index f80da50d84a5..95beaef6b66d 100644
> > --- a/drivers/infiniband/core/addr.c
> > +++ b/drivers/infiniband/core/addr.c
> > @@ -128,7 +128,7 @@ int rdma_translate_ip(struct sockaddr *addr,
> struct rdma_dev_addr *dev_addr,
> > int ret = -EADDRNOTAVAIL;
> >
> > if (dev_addr->bound_dev_if) {
> > - dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
> > + dev = dev_get_by_index(dev_addr->net, dev_addr-
> >bound_dev_if);
> > if (!dev)
> > return -ENODEV;
> > ret = rdma_copy_addr(dev_addr, dev, NULL);
> > @@ -137,9 +137,10 @@ int rdma_translate_ip(struct sockaddr *addr,
> struct rdma_dev_addr *dev_addr,
> > }
> >
> > switch (addr->sa_family) {
> > - case AF_INET:
> > - dev = ip_dev_find(&init_net,
> > - ((struct sockaddr_in *) addr)->sin_addr.s_addr);
> > + case AF_INET: {
> > + struct sockaddr_in *addr_in = (struct sockaddr_in *)addr;
> > +
> > + dev = ip_dev_find(dev_addr->net, addr_in->sin_addr.s_addr);
>
> I don't see the point of this change.
>
Note that we changed &init_net to be dev_addr->net .
The rest of the change here was done to avoid issues with checkpatch, as the line was getting really long.
> >
> > if (!dev)
> > return ret;
> > @@ -149,12 +150,12 @@ int rdma_translate_ip(struct sockaddr *addr,
> struct rdma_dev_addr *dev_addr,
> > *vlan_id = rdma_vlan_dev_vlan_id(dev);
> > dev_put(dev);
> > break;
> > -
> > + }
>
> closing } here ?
We opened a block in the beginning of this case ("case AF_INET: {"), we close it at the end of the case.
>
> > #if IS_ENABLED(CONFIG_IPV6)
> > case AF_INET6:
> > rcu_read_lock();
> > - for_each_netdev_rcu(&init_net, dev) {
> > - if (ipv6_chk_addr(&init_net,
> > + for_each_netdev_rcu(dev_addr->net, dev) {
> > + if (ipv6_chk_addr(dev_addr->net,
> > &((struct sockaddr_in6 *) addr)->sin6_addr,
> > dev, 1)) {
> > ret = rdma_copy_addr(dev_addr, dev, NULL);
> > @@ -236,7 +237,7 @@ static int addr4_resolve(struct sockaddr_in
> *src_in,
> > fl4.daddr = dst_ip;
> > fl4.saddr = src_ip;
> > fl4.flowi4_oif = addr->bound_dev_if;
> > - rt = ip_route_output_key(&init_net, &fl4);
> > + rt = ip_route_output_key(addr->net, &fl4);
> > if (IS_ERR(rt)) {
> > ret = PTR_ERR(rt);
> > goto out;
> > @@ -278,12 +279,13 @@ static int addr6_resolve(struct sockaddr_in6
> *src_in,
> > fl6.saddr = src_in->sin6_addr;
> > fl6.flowi6_oif = addr->bound_dev_if;
> >
> > - dst = ip6_route_output(&init_net, NULL, &fl6);
> > + dst = ip6_route_output(addr->net, NULL, &fl6);
> > if ((ret = dst->error))
> > goto put;
> >
> > if (ipv6_addr_any(&fl6.saddr)) {
> > - ret = ipv6_dev_get_saddr(&init_net, ip6_dst_idev(dst)->dev,
> > + ret = ipv6_dev_get_saddr(addr->net,
> > + ip6_dst_idev(dst)->dev,
> > &fl6.daddr, 0, &fl6.saddr);
> > if (ret)
> > goto put;
> > @@ -458,7 +460,7 @@ static void resolve_cb(int status, struct sockaddr
> *src_addr,
> > }
> >
> > int rdma_addr_find_dmac_by_grh(union ib_gid *sgid, union ib_gid
> *dgid, u8 *dmac,
> > - u16 *vlan_id)
> > + u16 *vlan_id, struct net *net)
> > {
> > int ret = 0;
> > struct rdma_dev_addr dev_addr;
> > @@ -481,6 +483,7 @@ int rdma_addr_find_dmac_by_grh(union ib_gid *sgid,
> union ib_gid *dgid, u8 *dmac,
> > return ret;
> >
> > memset(&dev_addr, 0, sizeof(dev_addr));
> > + dev_addr.net = net;
>
> Should be get_net() be used somewhere to grab a reference on the net
> namespace ?
>
Not needed, as dev_addr.net is used only inside this function. Assuming that the caller guarantees that the network namespace doesn't disappear until the function returns, there is no need to take a reference here. This kind of assumption makes sense, as otherwise we will not be able to use the argument at all.
> >
> > ctx.addr = &dev_addr;
> > init_completion(&ctx.comp);
> > @@ -492,7 +495,7 @@ int rdma_addr_find_dmac_by_grh(union ib_gid *sgid,
> union ib_gid *dgid, u8 *dmac,
> > wait_for_completion(&ctx.comp);
> >
> > memcpy(dmac, dev_addr.dst_dev_addr, ETH_ALEN);
> > - dev = dev_get_by_index(&init_net, dev_addr.bound_dev_if);
> > + dev = dev_get_by_index(net, dev_addr.bound_dev_if);
> > if (!dev)
> > return -ENODEV;
> > if (vlan_id)
> > @@ -502,7 +505,8 @@ int rdma_addr_find_dmac_by_grh(union ib_gid *sgid,
> union ib_gid *dgid, u8 *dmac,
> > }
> > EXPORT_SYMBOL(rdma_addr_find_dmac_by_grh);
> >
> > -int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> *vlan_id)
> > +int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> *vlan_id,
> > + struct net *net)
> > {
> > int ret = 0;
> > struct rdma_dev_addr dev_addr;
> > @@ -517,6 +521,7 @@ int rdma_addr_find_smac_by_sgid(union ib_gid
> *sgid, u8 *smac, u16 *vlan_id)
> > if (ret)
> > return ret;
> > memset(&dev_addr, 0, sizeof(dev_addr));
> > + dev_addr.net = net;
>
> get_net() ?
>
Same as before - used only in the function, caller must make sure it doesn't disappear.
> > ret = rdma_translate_ip(&gid_addr._sockaddr, &dev_addr, vlan_id);
> > if (ret)
> > return ret;
> > diff --git a/drivers/infiniband/core/cma.c
> b/drivers/infiniband/core/cma.c
> > index 6e5e11ca7702..aeb2417ec928 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -512,6 +512,7 @@ struct rdma_cm_id
> *rdma_create_id(rdma_cm_event_handler event_handler,
> > INIT_LIST_HEAD(&id_priv->listen_list);
> > INIT_LIST_HEAD(&id_priv->mc_list);
> > get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
> > + id_priv->id.route.addr.dev_addr.net = &init_net;
> >
> > return &id_priv->id;
> > }
> > @@ -637,7 +638,8 @@ static int cma_modify_qp_rtr(struct
> rdma_id_private *id_priv,
> > == RDMA_TRANSPORT_IB &&
> > rdma_port_get_link_layer(id_priv->id.device, id_priv-
> >id.port_num)
> > == IB_LINK_LAYER_ETHERNET) {
> > - ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL);
> > + ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL,
> > + &init_net);
> >
> > if (ret)
> > goto out;
> > diff --git a/drivers/infiniband/core/verbs.c
> b/drivers/infiniband/core/verbs.c
> > index f93eb8da7b5a..ca5c4dd8a67a 100644
> > --- a/drivers/infiniband/core/verbs.c
> > +++ b/drivers/infiniband/core/verbs.c
> > @@ -212,7 +212,9 @@ int ib_init_ah_from_wc(struct ib_device *device,
> u8 port_num, struct ib_wc *wc,
> > ah_attr->vlan_id = wc->vlan_id;
> > } else {
> > ret = rdma_addr_find_dmac_by_grh(&grh->dgid, &grh->sgid,
> > - ah_attr->dmac, &ah_attr->vlan_id);
> > + ah_attr->dmac,
> > + &ah_attr->vlan_id,
> > + &init_net);
> > if (ret)
> > return ret;
> > }
> > @@ -882,11 +884,15 @@ int ib_resolve_eth_l2_attrs(struct ib_qp *qp,
> > if (!(*qp_attr_mask & IB_QP_VID))
> > qp_attr->vlan_id = rdma_get_vlan_id(&sgid);
> > } else {
> > - ret = rdma_addr_find_dmac_by_grh(&sgid, &qp_attr-
> >ah_attr.grh.dgid,
> > - qp_attr->ah_attr.dmac, &qp_attr->vlan_id);
> > + ret = rdma_addr_find_dmac_by_grh(
> > + &sgid,
> > + &qp_attr->ah_attr.grh.dgid,
> > + qp_attr->ah_attr.dmac, &qp_attr->vlan_id,
> > + &init_net);
> > if (ret)
> > goto out;
> > - ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr->smac,
> NULL);
> > + ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr->smac,
> > + NULL, &init_net);
> > if (ret)
> > goto out;
> > }
> > diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> > index f3cc8c9e65ae..debaac2b6ee8 100644
> > --- a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> > +++ b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> > @@ -119,7 +119,8 @@ struct ib_ah *ocrdma_create_ah(struct ib_pd *ibpd,
> struct ib_ah_attr *attr)
> >
> > if (pd->uctx) {
> > status = rdma_addr_find_dmac_by_grh(&sgid, &attr->grh.dgid,
> > - attr->dmac, &attr->vlan_id);
> > + attr->dmac, &attr->vlan_id,
> > + &init_net);
> > if (status) {
> > pr_err("%s(): Failed to resolve dmac from gid."
> > "status = %d\n", __func__, status);
> > diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
> > index ce55906b54a0..40ccf8b83755 100644
> > --- a/include/rdma/ib_addr.h
> > +++ b/include/rdma/ib_addr.h
> > @@ -47,6 +47,7 @@
> > #include <rdma/ib_verbs.h>
> > #include <rdma/ib_pack.h>
> > #include <net/ipv6.h>
> > +#include <net/net_namespace.h>
> >
> > struct rdma_addr_client {
> > atomic_t refcount;
> > @@ -64,6 +65,16 @@ void rdma_addr_register_client(struct
> rdma_addr_client *client);
> > */
> > void rdma_addr_unregister_client(struct rdma_addr_client *client);
> >
> > +/**
> > + * struct rdma_dev_addr - Contains resolved RDMA hardware addresses
> > + * @src_dev_addr: Source MAC address.
> > + * @dst_dev_addr: Destination MAC address.
> > + * @broadcast: Broadcast address of the device.
> > + * @dev_type: The interface hardware type of the device.
> > + * @bound_dev_if: An optional device interface index.
> > + * @transport: The transport type used.
> > + * @net: Network namespace containing the bound_dev_if
> net_dev.
> > + */
> > struct rdma_dev_addr {
> > unsigned char src_dev_addr[MAX_ADDR_LEN];
> > unsigned char dst_dev_addr[MAX_ADDR_LEN];
> > @@ -71,11 +82,14 @@ struct rdma_dev_addr {
> > unsigned short dev_type;
> > int bound_dev_if;
> > enum rdma_transport_type transport;
> > + struct net *net;
> > };
> >
> > /**
> > * rdma_translate_ip - Translate a local IP address to an RDMA
> hardware
> > * address.
> > + *
> > + * The dev_addr->net field must be initialized.
> > */
> > int rdma_translate_ip(struct sockaddr *addr, struct rdma_dev_addr
> *dev_addr,
> > u16 *vlan_id);
> > @@ -90,7 +104,7 @@ int rdma_translate_ip(struct sockaddr *addr, struct
> rdma_dev_addr *dev_addr,
> > * @dst_addr: The destination address to resolve.
> > * @addr: A reference to a data location that will receive the
> resolved
> > * addresses. The data location must remain valid until the
> callback has
> > - * been invoked.
> > + * been invoked. The net field of the addr struct must be valid.
> > * @timeout_ms: Amount of time to wait for the address resolution to
> complete.
> > * @callback: Call invoked once address resolution has completed,
> timed out,
> > * or been canceled. A status of 0 indicates success.
> > @@ -110,9 +124,29 @@ int rdma_copy_addr(struct rdma_dev_addr
> *dev_addr, struct net_device *dev,
> >
> > int rdma_addr_size(struct sockaddr *addr);
> >
> > -int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> *vlan_id);
> > -int rdma_addr_find_dmac_by_grh(union ib_gid *sgid, union ib_gid
> *dgid, u8 *smac,
> > - u16 *vlan_id);
> > +/** rdma_addr_find_smac_by_sgid() - Find the src MAC and VLAN ID for
> a src GID
> > + * @sgid: Source GID to find the MAC and VLAN for.
> > + * @smac: A buffer to contain the resulting MAC address.
> > + * @vlan_id: Will contain the resulting VLAN ID.
> > + * @net: Network namespace to use for the address resolution.
> > + *
> > + * It is the caller's responsibility to keep the network namespace
> alive until
> > + * the function returns.
>
> Why ?
>
So that we could use the argument. Otherwise, we will need to have ugly code like:
------------------------
struct net *local_net = NULL;
rcu_read_lock();
for_each_net_rcu(local_net)
if (local_net == net)
break;
if (local_net == net)
get_net(local_net);
else
local_net = NULL;
rcu_read_unlock();
------------------------
however, the callers (in following patches), can easily ensure that the network namespace is here to stay. This is much easier to understand and maintain.
> > + */
> > +int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> *vlan_id,
> > + struct net *net);
> > +/** rdma_addr_find_dmac_by_grh() - Find the dst MAC and VLAN ID for a
> GID pair
> > + * @sgid: Source GID to use for the search.
> > + * @dgid: Destination GID to find the details for.
> > + * @dmac: Contains the resulting destination MAC address.
> > + * @vlan_id: Contains the resulting VLAN ID.
> > + * @net: Network namespace to use for the address resolution.
> > + *
> > + * It is the caller's responsibility to keep the network namespace
> alive until
> > + * the function returns.
>
> Why ?
>
See above.
> > + */
> > +int rdma_addr_find_dmac_by_grh(union ib_gid *sgid, union ib_gid
> *dgid, u8 *dmac,
> > + u16 *vlan_id, struct net *net);
> >
> > static inline u16 ib_addr_get_pkey(struct rdma_dev_addr *dev_addr)
> > {
> > @@ -182,7 +216,7 @@ static inline void iboe_addr_get_sgid(struct
> rdma_dev_addr *dev_addr,
> > struct net_device *dev;
> > struct in_device *ip4;
> >
> > - dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
> > + dev = dev_get_by_index(dev_addr->net, dev_addr->bound_dev_if);
> > if (dev) {
> > ip4 = (struct in_device *)dev->ip_ptr;
> > if (ip4 && ip4->ifa_list && ip4->ifa_list->ifa_address)
>
>
> I believe this patch lack proper reference counting in form of
> get_net() / put_net(), but cannot say for sure.
>
If you could point to specific issues or race conditions, that would be great.
We have thoroughly tested and reviewed the code, and couldn't find any such issues with the submitted patches.
--Shachar
^ permalink raw reply
* RE: [PATCH for-next 08/10] IB/cma: Add support for network namespaces
From: Shachar Raindel @ 2015-02-01 14:16 UTC (permalink / raw)
To: Yann Droneaud
Cc: roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Liran Liss,
Guy Shapiro, Haggai Eran, Yotam Kenneth
In-Reply-To: <1422798272.3030.48.camel-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 19390 bytes --]
> -----Original Message-----
> From: Yann Droneaud [mailto:ydroneaud@opteya.com]
> Sent: Sunday, February 01, 2015 3:45 PM
> To: Shachar Raindel
> Cc: roland@kernel.org; sean.hefty@intel.com; linux-rdma@vger.kernel.org;
> netdev@vger.kernel.org; Liran Liss; Guy Shapiro; Haggai Eran; Yotam
> Kenneth
> Subject: Re: [PATCH for-next 08/10] IB/cma: Add support for network
> namespaces
>
> Hi,
>
> Le dimanche 01 février 2015 à 13:28 +0200, Shachar Raindel a écrit :
> > From: Guy Shapiro <guysh@mellanox.com>
> >
> > Add support for network namespaces in the ib_cma module. This is
> > accomplished by:
> >
> > 1. Adding network namespace parameter for rdma_create_id. This
> parameter is used
> > to populate the network namespace field in rdma_id_private.
> rdma_create_id
> > keeps a reference on the network namespace.
> > 2. Using the network namespace from the rdma_id instead of init_net
> inside of
> > ib_cma.
> > 3. Decrementing the reference count for the appropriate network
> namespace when
> > calling rdma_destroy_id.
> >
> > In order to preserve the current behavior init_net is passed when
> calling from
> > other modules.
> >
> > Signed-off-by: Guy Shapiro <guysh@mellanox.com>
> > Signed-off-by: Haggai Eran <haggaie@mellanox.com>
> > Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> > Signed-off-by: Shachar Raindel <raindel@mellanox.com>
> >
> > ---
> > drivers/infiniband/core/cma.c | 52
> +++++++++++++---------
> > drivers/infiniband/core/ucma.c | 3 +-
> > drivers/infiniband/ulp/iser/iser_verbs.c | 2 +-
> > drivers/infiniband/ulp/isert/ib_isert.c | 2 +-
> > .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h | 4 +-
> > include/rdma/rdma_cm.h | 6 ++-
> > net/9p/trans_rdma.c | 2 +-
> > net/rds/ib.c | 2 +-
> > net/rds/ib_cm.c | 2 +-
> > net/rds/iw.c | 2 +-
> > net/rds/iw_cm.c | 2 +-
> > net/rds/rdma_transport.c | 2 +-
> > net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 +-
> > net/sunrpc/xprtrdma/verbs.c | 3 +-
> > 14 files changed, 52 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c
> b/drivers/infiniband/core/cma.c
> > index 022b0d0a51cc..f6379b38b366 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -540,7 +540,8 @@ static int cma_disable_callback(struct
> rdma_id_private *id_priv,
> >
> > struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler
> event_handler,
> > void *context, enum rdma_port_space ps,
> > - enum ib_qp_type qp_type)
> > + enum ib_qp_type qp_type,
> > + struct net *net)
> > {
> > struct rdma_id_private *id_priv;
> >
> > @@ -562,7 +563,7 @@ struct rdma_cm_id
> *rdma_create_id(rdma_cm_event_handler event_handler,
> > INIT_LIST_HEAD(&id_priv->listen_list);
> > INIT_LIST_HEAD(&id_priv->mc_list);
> > get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
> > - id_priv->id.route.addr.dev_addr.net = &init_net;
> > + id_priv->id.route.addr.dev_addr.net = get_net(net);
> >
> > return &id_priv->id;
> > }
> > @@ -689,7 +690,7 @@ static int cma_modify_qp_rtr(struct
> rdma_id_private *id_priv,
> > rdma_port_get_link_layer(id_priv->id.device, id_priv-
> >id.port_num)
> > == IB_LINK_LAYER_ETHERNET) {
> > ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL,
> > - &init_net);
> > + id_priv->id.route.addr.dev_addr.net);
> >
> > if (ret)
> > goto out;
> > @@ -953,6 +954,7 @@ static void cma_cancel_operation(struct
> rdma_id_private *id_priv,
> > static void cma_release_port(struct rdma_id_private *id_priv)
> > {
> > struct rdma_bind_list *bind_list = id_priv->bind_list;
> > + struct net *net = id_priv->id.route.addr.dev_addr.net;
> >
> > if (!bind_list)
> > return;
> > @@ -960,7 +962,7 @@ static void cma_release_port(struct
> rdma_id_private *id_priv)
> > mutex_lock(&lock);
> > hlist_del(&id_priv->node);
> > if (hlist_empty(&bind_list->owners)) {
> > - cma_ps_remove(bind_list->ps, &init_net, bind_list->port);
> > + cma_ps_remove(bind_list->ps, net, bind_list->port);
> > kfree(bind_list);
> > }
> > mutex_unlock(&lock);
> > @@ -1029,6 +1031,7 @@ void rdma_destroy_id(struct rdma_cm_id *id)
> > cma_deref_id(id_priv->id.context);
> >
> > kfree(id_priv->id.route.path_rec);
> > + put_net(id_priv->id.route.addr.dev_addr.net);
> > kfree(id_priv);
> > }
> > EXPORT_SYMBOL(rdma_destroy_id);
> > @@ -1156,7 +1159,8 @@ static struct rdma_id_private
> *cma_new_conn_id(struct rdma_cm_id *listen_id,
> > int ret;
> >
> > id = rdma_create_id(listen_id->event_handler, listen_id->context,
> > - listen_id->ps, ib_event->param.req_rcvd.qp_type);
> > + listen_id->ps, ib_event->param.req_rcvd.qp_type,
> > + listen_id->route.addr.dev_addr.net);
> > if (IS_ERR(id))
> > return NULL;
> >
> > @@ -1201,10 +1205,11 @@ static struct rdma_id_private
> *cma_new_udp_id(struct rdma_cm_id *listen_id,
> > {
> > struct rdma_id_private *id_priv;
> > struct rdma_cm_id *id;
> > + struct net *net = listen_id->route.addr.dev_addr.net;
> > int ret;
> >
> > id = rdma_create_id(listen_id->event_handler, listen_id->context,
> > - listen_id->ps, IB_QPT_UD);
> > + listen_id->ps, IB_QPT_UD, net);
> > if (IS_ERR(id))
> > return NULL;
> >
> > @@ -1455,7 +1460,8 @@ static int iw_conn_req_handler(struct iw_cm_id
> *cm_id,
> > /* Create a new RDMA id for the new IW CM ID */
> > new_cm_id = rdma_create_id(listen_id->id.event_handler,
> > listen_id->id.context,
> > - RDMA_PS_TCP, IB_QPT_RC);
> > + RDMA_PS_TCP, IB_QPT_RC,
> > + listen_id->id.route.addr.dev_addr.net);
> > if (IS_ERR(new_cm_id)) {
> > ret = -ENOMEM;
> > goto out;
> > @@ -1528,11 +1534,11 @@ static int cma_ib_listen(struct
> rdma_id_private *id_priv)
> > struct ib_cm_compare_data compare_data;
> > struct sockaddr *addr;
> > struct ib_cm_id *id;
> > + struct net *net = id_priv->id.route.addr.dev_addr.net;
> > __be64 svc_id;
> > int ret;
> >
> > - id = ib_create_cm_id(id_priv->id.device, cma_req_handler, id_priv,
> > - &init_net);
> > + id = ib_create_cm_id(id_priv->id.device, cma_req_handler, id_priv,
> net);
> > if (IS_ERR(id))
> > return PTR_ERR(id);
> >
> > @@ -1596,6 +1602,7 @@ static void cma_listen_on_dev(struct
> rdma_id_private *id_priv,
> > {
> > struct rdma_id_private *dev_id_priv;
> > struct rdma_cm_id *id;
> > + struct net *net = id_priv->id.route.addr.dev_addr.net;
> > int ret;
> >
> > if (cma_family(id_priv) == AF_IB &&
> > @@ -1603,7 +1610,7 @@ static void cma_listen_on_dev(struct
> rdma_id_private *id_priv,
> > return;
> >
> > id = rdma_create_id(cma_listen_handler, id_priv, id_priv->id.ps,
> > - id_priv->id.qp_type);
> > + id_priv->id.qp_type, net);
> > if (IS_ERR(id))
> > return;
> >
> > @@ -2283,7 +2290,8 @@ static int cma_alloc_port(struct radix_tree_root
> *ps,
> > if (!bind_list)
> > return -ENOMEM;
> >
> > - ret = cma_ps_alloc(ps, &init_net, bind_list, snum);
> > + ret = cma_ps_alloc(ps, id_priv->id.route.addr.dev_addr.net,
> bind_list,
> > + snum);
> > if (ret < 0)
> > goto err;
> >
> > @@ -2302,13 +2310,14 @@ static int cma_alloc_any_port(struct
> radix_tree_root *ps,
> > static unsigned int last_used_port;
> > int low, high, remaining;
> > unsigned int rover;
> > + struct net *net = id_priv->id.route.addr.dev_addr.net;
> >
> > - inet_get_local_port_range(&init_net, &low, &high);
> > + inet_get_local_port_range(net, &low, &high);
> > remaining = (high - low) + 1;
> > rover = prandom_u32() % remaining + low;
> > retry:
> > if (last_used_port != rover &&
> > - !cma_ps_find(ps, &init_net, (unsigned short)rover)) {
> > + !cma_ps_find(ps, net, (unsigned short)rover)) {
> > int ret = cma_alloc_port(ps, id_priv, rover);
> > /*
> > * Remember previously used port number in order to avoid
> > @@ -2376,7 +2385,7 @@ static int cma_use_port(struct radix_tree_root
> *ps,
> > if (snum < PROT_SOCK && !capable(CAP_NET_BIND_SERVICE))
> > return -EACCES;
> >
> > - bind_list = cma_ps_find(ps, &init_net, snum);
> > + bind_list = cma_ps_find(ps, id_priv->id.route.addr.dev_addr.net,
> snum);
> > if (!bind_list) {
> > ret = cma_alloc_port(ps, id_priv, snum);
> > } else {
> > @@ -2573,8 +2582,11 @@ int rdma_bind_addr(struct rdma_cm_id *id,
> struct sockaddr *addr)
> > if (addr->sa_family == AF_INET)
> > id_priv->afonly = 1;
> > #if IS_ENABLED(CONFIG_IPV6)
> > - else if (addr->sa_family == AF_INET6)
> > - id_priv->afonly = init_net.ipv6.sysctl.bindv6only;
> > + else if (addr->sa_family == AF_INET6) {
> > + struct net *net = id_priv->id.route.addr.dev_addr.net;
> > +
> > + id_priv->afonly = net->ipv6.sysctl.bindv6only;
> > + }
> > #endif
> > }
> > ret = cma_get_port(id_priv);
> > @@ -2687,7 +2699,7 @@ static int cma_resolve_ib_udp(struct
> rdma_id_private *id_priv,
> > }
> >
> > id = ib_create_cm_id(id_priv->id.device, cma_sidr_rep_handler,
> > - id_priv, &init_net);
> > + id_priv, id_priv->id.route.addr.dev_addr.net);
> > if (IS_ERR(id)) {
> > ret = PTR_ERR(id);
> > goto out;
> > @@ -2737,7 +2749,7 @@ static int cma_connect_ib(struct rdma_id_private
> *id_priv,
> > conn_param->private_data_len);
> >
> > id = ib_create_cm_id(id_priv->id.device, cma_ib_handler, id_priv,
> > - &init_net);
> > + id_priv->id.route.addr.dev_addr.net);
> > if (IS_ERR(id)) {
> > ret = PTR_ERR(id);
> > goto out;
> > @@ -3387,6 +3399,7 @@ static int cma_netdev_change(struct net_device
> *ndev, struct rdma_id_private *id
> > dev_addr = &id_priv->id.route.addr.dev_addr;
> >
> > if ((dev_addr->bound_dev_if == ndev->ifindex) &&
> > + (dev_net(ndev) == dev_addr->net) &&
>
> net_eq() ?
The original code (below) contained the same comparison style.
Will fix in next iteration to use net_eq.
>
> > memcmp(dev_addr->src_dev_addr, ndev->dev_addr, ndev->addr_len))
> {
> > printk(KERN_INFO "RDMA CM addr change for ndev %s used by id
> %p\n",
> > ndev->name, &id_priv->id);
> > @@ -3412,9 +3425,6 @@ static int cma_netdev_callback(struct
> notifier_block *self, unsigned long event,
> > struct rdma_id_private *id_priv;
> > int ret = NOTIFY_DONE;
> >
> > - if (dev_net(ndev) != &init_net)
> > - return NOTIFY_DONE;
> > -
> > if (event != NETDEV_BONDING_FAILOVER)
> > return NOTIFY_DONE;
> >
> > diff --git a/drivers/infiniband/core/ucma.c
> b/drivers/infiniband/core/ucma.c
> > index 56a4b7ca7ee3..de755f2c6166 100644
> > --- a/drivers/infiniband/core/ucma.c
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -391,7 +391,8 @@ static ssize_t ucma_create_id(struct ucma_file
> *file, const char __user *inbuf,
> > return -ENOMEM;
> >
> > ctx->uid = cmd.uid;
> > - ctx->cm_id = rdma_create_id(ucma_event_handler, ctx, cmd.ps,
> qp_type);
> > + ctx->cm_id = rdma_create_id(ucma_event_handler, ctx, cmd.ps,
> qp_type,
> > + &init_net);
> > if (IS_ERR(ctx->cm_id)) {
> > ret = PTR_ERR(ctx->cm_id);
> > goto err1;
> > diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c
> b/drivers/infiniband/ulp/iser/iser_verbs.c
> > index 695a2704bd43..d4e9c639ad2f 100644
> > --- a/drivers/infiniband/ulp/iser/iser_verbs.c
> > +++ b/drivers/infiniband/ulp/iser/iser_verbs.c
> > @@ -949,7 +949,7 @@ int iser_connect(struct iser_conn *iser_conn,
> >
> > ib_conn->cma_id = rdma_create_id(iser_cma_handler,
> > (void *)iser_conn,
> > - RDMA_PS_TCP, IB_QPT_RC);
> > + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> > if (IS_ERR(ib_conn->cma_id)) {
> > err = PTR_ERR(ib_conn->cma_id);
> > iser_err("rdma_create_id failed: %d\n", err);
> > diff --git a/drivers/infiniband/ulp/isert/ib_isert.c
> b/drivers/infiniband/ulp/isert/ib_isert.c
> > index dafb3c531f96..44a6fff8dc79 100644
> > --- a/drivers/infiniband/ulp/isert/ib_isert.c
> > +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> > @@ -2960,7 +2960,7 @@ isert_setup_id(struct isert_np *isert_np)
> > isert_dbg("ksockaddr: %p, sa: %p\n", &np->np_sockaddr, sa);
> >
> > id = rdma_create_id(isert_cma_handler, isert_np,
> > - RDMA_PS_TCP, IB_QPT_RC);
> > + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> > if (IS_ERR(id)) {
> > isert_err("rdma_create_id() failed: %ld\n", PTR_ERR(id));
> > ret = PTR_ERR(id);
> > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
> b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
> > index b02b4ec1e29d..128de4eb0959 100644
> > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
> > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
> > @@ -125,7 +125,9 @@ extern kib_tunables_t kiblnd_tunables;
> > IBLND_CREDIT_HIGHWATER_V1 : \
> > *kiblnd_tunables.kib_peercredits_hiw) /* when
> eagerly to return credits */
> >
> > -#define kiblnd_rdma_create_id(cb, dev, ps, qpt) rdma_create_id(cb,
> dev, ps, qpt)
> > +#define kiblnd_rdma_create_id(cb, dev, ps, qpt) rdma_create_id(cb,
> dev, \
> > + ps, qpt, \
> > + &init_net)
> >
> > static inline int
> > kiblnd_concurrent_sends_v1(void)
> > diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
> > index 1ed2088dc9f5..3953e9c8bc94 100644
> > --- a/include/rdma/rdma_cm.h
> > +++ b/include/rdma/rdma_cm.h
> > @@ -163,10 +163,14 @@ struct rdma_cm_id {
> > * @context: User specified context associated with the id.
> > * @ps: RDMA port space.
> > * @qp_type: type of queue pair associated with the id.
> > + * @net: The network namespace in which to create the new id.
> > + *
> > + * The id holds a reference on the network namespace until it is
> destroyed.
> > */
> > struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler
> event_handler,
> > void *context, enum rdma_port_space ps,
> > - enum ib_qp_type qp_type);
> > + enum ib_qp_type qp_type,
> > + struct net *net);
> >
> > /**
> > * rdma_destroy_id - Destroys an RDMA identifier.
> > diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
> > index 14ad43b5cf89..577fd3129bcf 100644
> > --- a/net/9p/trans_rdma.c
> > +++ b/net/9p/trans_rdma.c
> > @@ -635,7 +635,7 @@ rdma_create_trans(struct p9_client *client, const
> char *addr, char *args)
> >
> > /* Create the RDMA CM ID */
> > rdma->cm_id = rdma_create_id(p9_cm_event_handler, client,
> RDMA_PS_TCP,
> > - IB_QPT_RC);
> > + IB_QPT_RC, &init_net);
> > if (IS_ERR(rdma->cm_id))
> > goto error;
> >
> > diff --git a/net/rds/ib.c b/net/rds/ib.c
> > index ba2dffeff608..cc137f523248 100644
> > --- a/net/rds/ib.c
> > +++ b/net/rds/ib.c
> > @@ -326,7 +326,7 @@ static int rds_ib_laddr_check(__be32 addr)
> > /* Create a CMA ID and try to bind it. This catches both
> > * IB and iWARP capable NICs.
> > */
> > - cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC);
> > + cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC,
> &init_net);
> > if (IS_ERR(cm_id))
> > return PTR_ERR(cm_id);
> >
> > diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
> > index 31b74f5e61ad..d19b91296ddc 100644
> > --- a/net/rds/ib_cm.c
> > +++ b/net/rds/ib_cm.c
> > @@ -584,7 +584,7 @@ int rds_ib_conn_connect(struct rds_connection
> *conn)
> > /* XXX I wonder what affect the port space has */
> > /* delegate cm event handler to rdma_transport */
> > ic->i_cm_id = rdma_create_id(rds_rdma_cm_event_handler, conn,
> > - RDMA_PS_TCP, IB_QPT_RC);
> > + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> > if (IS_ERR(ic->i_cm_id)) {
> > ret = PTR_ERR(ic->i_cm_id);
> > ic->i_cm_id = NULL;
> > diff --git a/net/rds/iw.c b/net/rds/iw.c
> > index 589935661d66..8501b73ed12f 100644
> > --- a/net/rds/iw.c
> > +++ b/net/rds/iw.c
> > @@ -227,7 +227,7 @@ static int rds_iw_laddr_check(__be32 addr)
> > /* Create a CMA ID and try to bind it. This catches both
> > * IB and iWARP capable NICs.
> > */
> > - cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC);
> > + cm_id = rdma_create_id(NULL, NULL, RDMA_PS_TCP, IB_QPT_RC,
> &init_net);
> > if (IS_ERR(cm_id))
> > return PTR_ERR(cm_id);
> >
> > diff --git a/net/rds/iw_cm.c b/net/rds/iw_cm.c
> > index a91e1db62ee6..e5ee2d562a60 100644
> > --- a/net/rds/iw_cm.c
> > +++ b/net/rds/iw_cm.c
> > @@ -521,7 +521,7 @@ int rds_iw_conn_connect(struct rds_connection
> *conn)
> > /* XXX I wonder what affect the port space has */
> > /* delegate cm event handler to rdma_transport */
> > ic->i_cm_id = rdma_create_id(rds_rdma_cm_event_handler, conn,
> > - RDMA_PS_TCP, IB_QPT_RC);
> > + RDMA_PS_TCP, IB_QPT_RC, &init_net);
> > if (IS_ERR(ic->i_cm_id)) {
> > ret = PTR_ERR(ic->i_cm_id);
> > ic->i_cm_id = NULL;
> > diff --git a/net/rds/rdma_transport.c b/net/rds/rdma_transport.c
> > index 6cd9d1deafc3..066b60b27b12 100644
> > --- a/net/rds/rdma_transport.c
> > +++ b/net/rds/rdma_transport.c
> > @@ -160,7 +160,7 @@ static int rds_rdma_listen_init(void)
> > int ret;
> >
> > cm_id = rdma_create_id(rds_rdma_cm_event_handler, NULL,
> RDMA_PS_TCP,
> > - IB_QPT_RC);
> > + IB_QPT_RC, &init_net);
> > if (IS_ERR(cm_id)) {
> > ret = PTR_ERR(cm_id);
> > printk(KERN_ERR "RDS/RDMA: failed to setup listener, "
> > diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > index 4e618808bc98..e3b246e305f9 100644
> > --- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > +++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
> > @@ -701,7 +701,7 @@ static struct svc_xprt *svc_rdma_create(struct
> svc_serv *serv,
> > xprt = &cma_xprt->sc_xprt;
> >
> > listen_id = rdma_create_id(rdma_listen_handler, cma_xprt,
> RDMA_PS_TCP,
> > - IB_QPT_RC);
> > + IB_QPT_RC, &init_net);
> > if (IS_ERR(listen_id)) {
> > ret = PTR_ERR(listen_id);
> > dprintk("svcrdma: rdma_create_id failed = %d\n", ret);
> > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> > index c98e40643910..f574e77165f4 100644
> > --- a/net/sunrpc/xprtrdma/verbs.c
> > +++ b/net/sunrpc/xprtrdma/verbs.c
> > @@ -483,7 +483,8 @@ rpcrdma_create_id(struct rpcrdma_xprt *xprt,
> >
> > init_completion(&ia->ri_done);
> >
> > - id = rdma_create_id(rpcrdma_conn_upcall, xprt, RDMA_PS_TCP,
> IB_QPT_RC);
> > + id = rdma_create_id(rpcrdma_conn_upcall, xprt, RDMA_PS_TCP,
> IB_QPT_RC,
> > + &init_net);
> > if (IS_ERR(id)) {
> > rc = PTR_ERR(id);
> > dprintk("RPC: %s: rdma_create_id() failed %i\n",
>
Thanks,
--Shachar
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±ÙšŠ{ayº\x1dʇڙë,j\a¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš
^ permalink raw reply
* RE: [PATCH for-next 02/10] IB/core: Pass network namespace as a parameter to relevant functions
From: Shachar Raindel @ 2015-02-01 14:10 UTC (permalink / raw)
To: Yann Droneaud
Cc: roland@kernel.org, sean.hefty@intel.com,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org, Liran Liss,
Guy Shapiro, Haggai Eran, Yotam Kenneth
In-Reply-To: <1422793610.3030.39.camel@opteya.com>
Hi,
> -----Original Message-----
> From: Yann Droneaud [mailto:ydroneaud@opteya.com]
> Sent: Sunday, February 01, 2015 2:27 PM
> To: Shachar Raindel
> Cc: roland@kernel.org; sean.hefty@intel.com; linux-rdma@vger.kernel.org;
> netdev@vger.kernel.org; Liran Liss; Guy Shapiro; Haggai Eran; Yotam
> Kenneth
> Subject: Re: [PATCH for-next 02/10] IB/core: Pass network namespace as a
> parameter to relevant functions
>
> Hi,
>
> Le dimanche 01 février 2015 à 13:28 +0200, Shachar Raindel a écrit :
> > From: Guy Shapiro <guysh@mellanox.com>
> >
> > Add network namespace parameters for the address related ib_core
> > functions. The parameter is passed to lower level function, instead of
> > &init_net, so things are done in the correct namespace.
> >
> > For now pass &init_net on every caller.
> > Callers that will pass &init_net permanently are marked with an
> > appropriate comment.
> >
> > Signed-off-by: Haggai Eran <haggaie@mellanox.com>
> > Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> > Signed-off-by: Shachar Raindel <raindel@mellanox.com>
> > Signed-off-by: Guy Shapiro <guysh@mellanox.com>
> >
> > ---
> > drivers/infiniband/core/agent.c | 4 +++-
> > drivers/infiniband/core/cm.c | 9 +++++++--
> > drivers/infiniband/core/mad_rmpp.c | 10 ++++++++--
> > drivers/infiniband/core/user_mad.c | 4 +++-
> > drivers/infiniband/core/verbs.c | 10 ++++++----
> > drivers/infiniband/ulp/srpt/ib_srpt.c | 3 ++-
> > include/rdma/ib_verbs.h | 15 +++++++++++++--
> > 7 files changed, 42 insertions(+), 13 deletions(-)
>
> > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > index 0d74f1de99aa..dd4c80cea8d3 100644
> > --- a/include/rdma/ib_verbs.h
> > +++ b/include/rdma/ib_verbs.h
> > @@ -48,6 +48,7 @@
> > #include <linux/rwsem.h>
> > #include <linux/scatterlist.h>
> > #include <linux/workqueue.h>
> > +#include <net/net_namespace.h>
> > #include <uapi/linux/if_ether.h>
> >
> > #include <linux/atomic.h>
> > @@ -1801,9 +1802,14 @@ struct ib_ah *ib_create_ah(struct ib_pd *pd,
> struct ib_ah_attr *ah_attr);
> > * ignored unless the work completion indicates that the GRH is
> valid.
> > * @ah_attr: Returned attributes that can be used when creating an
> address
> > * handle for replying to the message.
> > + * @net: The network namespace to use for address resolution.
> > + *
> > + * It is the caller's responsibility to make sure the network
> namespace is
> > + * alive until the function returns.
>
> Why ?
>
For the same reason we described in the previous patch. It is nearly impossible to code if your function parameters are not guaranteed to be alive during the call to your function.
> > */
> > int ib_init_ah_from_wc(struct ib_device *device, u8 port_num, struct
> ib_wc *wc,
> > - struct ib_grh *grh, struct ib_ah_attr *ah_attr);
> > + struct ib_grh *grh, struct ib_ah_attr *ah_attr,
> > + struct net *net);
> >
> > /**
> > * ib_create_ah_from_wc - Creates an address handle associated with
> the
> > @@ -1813,12 +1819,17 @@ int ib_init_ah_from_wc(struct ib_device
> *device, u8 port_num, struct ib_wc *wc,
> > * @grh: References the received global route header. This parameter
> is
> > * ignored unless the work completion indicates that the GRH is
> valid.
> > * @port_num: The outbound port number to associate with the address.
> > + * @net: The network namespace to use for address resolution.
> > *
> > * The address handle is used to reference a local or global
> destination
> > * in all UD QP post sends.
> > + *
> > + * It is the caller's responsibility to make sure the network
> namespace is
> > + * alive until the function returns.
>
> Why ?
>
For the same reason we described in the previous patch. It is nearly impossible to code if your function parameters are not guaranteed to be alive during the call to your function.
Thanks,
--Shachar
^ permalink raw reply
* RE: [PATCH for-next 05/10] IB/cm,cma: Move RDMA IP CM private-data parsing code from ib_cma to ib_cm
From: Shachar Raindel @ 2015-02-01 14:29 UTC (permalink / raw)
To: Yann Droneaud
Cc: roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Liran Liss,
Guy Shapiro, Haggai Eran, Yotam Kenneth
In-Reply-To: <1422795359.3030.43.camel-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
> -----Original Message-----
> From: Yann Droneaud [mailto:ydroneaud@opteya.com]
> Sent: Sunday, February 01, 2015 2:56 PM
> To: Shachar Raindel
> Cc: roland@kernel.org; sean.hefty@intel.com; linux-rdma@vger.kernel.org;
> netdev@vger.kernel.org; Liran Liss; Guy Shapiro; Haggai Eran; Yotam
> Kenneth
> Subject: Re: [PATCH for-next 05/10] IB/cm,cma: Move RDMA IP CM private-
> data parsing code from ib_cma to ib_cm
>
> Le dimanche 01 février 2015 à 13:28 +0200, Shachar Raindel a écrit :
> > From: Guy Shapiro <guysh@mellanox.com>
> >
> > When receiving a connection request, ib_cm needs to associate the
> request with
> > a network namespace. To do this, it needs to know the request's
> destination
> > IP. For this the RDMA IP CM packet formatting functionality needs to
> be
> > exposed to ib_cm.
> >
> > This patch merely moves the RDMA IP CM data formatting and parsing
> functions
> > to be part of ib_cm. The following patch will utilize the new
> knowledge to
> > look-up the appropriate namespace. Each namespace maintains an
> independent
> > table of RDMA CM service IDs, allowing isolation and separation
> between the
> > network namespaces.
> >
> > When creating a new incoming connection ID, the code in
> cm_save_ip_info can no
> > longer rely on the listener's private data to find the port number, so
> it
> > reads it from the requested service ID. This required saving the
> service ID in
> > cm_format_paths_from_req.
> >
> > Signed-off-by: Guy Shapiro <guysh@mellanox.com>
> > Signed-off-by: Haggai Eran <haggaie@mellanox.com>
> > Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> > Signed-off-by: Shachar Raindel <raindel@mellanox.com>
> >
> > ---
> > drivers/infiniband/core/cm.c | 167
> ++++++++++++++++++++++++++++++++++++++++++
> > drivers/infiniband/core/cma.c | 166 +++++----------------------------
> --------
> > include/rdma/ib_cm.h | 46 ++++++++++++
> > 3 files changed, 231 insertions(+), 148 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cm.c
> b/drivers/infiniband/core/cm.c
> > index 5a45cb76c43e..5cc1a4aa9728 100644
> > --- a/drivers/infiniband/core/cm.c
> > +++ b/drivers/infiniband/core/cm.c
> > @@ -51,6 +51,7 @@
> >
> > #include <rdma/ib_cache.h>
> > #include <rdma/ib_cm.h>
> > +#include <rdma/ib.h>
> > #include "cm_msgs.h"
> >
> > MODULE_AUTHOR("Sean Hefty");
> > @@ -701,6 +702,170 @@ static void cm_reject_sidr_req(struct
> cm_id_private *cm_id_priv,
> > ib_send_cm_sidr_rep(&cm_id_priv->id, ¶m);
> > }
> >
> > +static inline u8 cm_get_ip_ver(struct cm_hdr *hdr)
> > +{
> > + return hdr->ip_version >> 4;
> > +}
> > +
> > +void cm_set_ip_ver(struct cm_hdr *hdr, u8 ip_ver)
> > +{
> > + hdr->ip_version = (ip_ver << 4) | (hdr->ip_version & 0xF);
> > +}
> > +EXPORT_SYMBOL(cm_set_ip_ver);
> > +
>
> That can be defined as an inline function in header.
>
While you are technically correct (the function is less than 3 lines, and therefore you can inline it), the performance gain from inlining it is negligible. At the same time, having a nice block where all of the RDMA IP CM protocol serialization lives make the code much easier to maintain.
> > +int cm_format_hdr(void *hdr, int family,
> > + struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr)
> > +{
> > + struct cm_hdr *cm_hdr;
> > +
> > + cm_hdr = hdr;
> > + cm_hdr->cm_version = RDMA_IP_CM_VERSION;
> > + if (family == AF_INET) {
> > + struct sockaddr_in *src4, *dst4;
> > +
> > + src4 = (struct sockaddr_in *)src_addr;
> > + dst4 = (struct sockaddr_in *)dst_addr;
> > +
> > + cm_set_ip_ver(cm_hdr, 4);
> > + cm_hdr->src_addr.ip4.addr = src4->sin_addr.s_addr;
> > + cm_hdr->dst_addr.ip4.addr = dst4->sin_addr.s_addr;
> > + cm_hdr->port = src4->sin_port;
> > + } else if (family == AF_INET6) {
> > + struct sockaddr_in6 *src6, *dst6;
> > +
> > + src6 = (struct sockaddr_in6 *)src_addr;
> > + dst6 = (struct sockaddr_in6 *)dst_addr;
> > +
> > + cm_set_ip_ver(cm_hdr, 6);
> > + cm_hdr->src_addr.ip6 = src6->sin6_addr;
> > + cm_hdr->dst_addr.ip6 = dst6->sin6_addr;
> > + cm_hdr->port = src6->sin6_port;
> > + }
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(cm_format_hdr);
> > +
> > +static void cm_save_ib_info(struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr,
> > + struct ib_sa_path_rec *path)
> > +{
> > + struct sockaddr_ib *ib;
> > +
> > + if (src_addr) {
> > + ib = (struct sockaddr_ib *)src_addr;
> > + ib->sib_family = AF_IB;
> > + ib->sib_pkey = path->pkey;
> > + ib->sib_flowinfo = path->flow_label;
> > + memcpy(&ib->sib_addr, &path->sgid, 16);
> > + ib->sib_sid = path->service_id;
> > + ib->sib_sid_mask = cpu_to_be64(0xffffffffffffffffULL);
> > + ib->sib_scope_id = 0;
> > + }
> > + if (dst_addr) {
> > + ib = (struct sockaddr_ib *)dst_addr;
> > + ib->sib_family = AF_IB;
> > + ib->sib_pkey = path->pkey;
> > + ib->sib_flowinfo = path->flow_label;
> > + memcpy(&ib->sib_addr, &path->dgid, 16);
> > + }
> > +}
> > +
> > +static void cm_save_ip6_info(struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr,
> > + struct cm_hdr *hdr,
> > + __be16 local_port)
> > +{
> > + struct sockaddr_in6 *ip6;
> > +
> > + if (src_addr) {
> > + ip6 = (struct sockaddr_in6 *)src_addr;
> > + ip6->sin6_family = AF_INET6;
> > + ip6->sin6_addr = hdr->dst_addr.ip6;
> > + ip6->sin6_port = local_port;
> > + }
> > +
> > + if (dst_addr) {
> > + ip6 = (struct sockaddr_in6 *)dst_addr;
> > + ip6->sin6_family = AF_INET6;
> > + ip6->sin6_addr = hdr->src_addr.ip6;
> > + ip6->sin6_port = hdr->port;
> > + }
> > +}
> > +
> > +static void cm_save_ip4_info(struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr,
> > + struct cm_hdr *hdr,
> > + __be16 local_port)
> > +{
> > + struct sockaddr_in *ip4;
> > +
> > + if (src_addr) {
> > + ip4 = (struct sockaddr_in *)src_addr;
> > + ip4->sin_family = AF_INET;
> > + ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
> > + ip4->sin_port = local_port;
> > + }
> > +
> > + if (dst_addr) {
> > + ip4 = (struct sockaddr_in *)dst_addr;
> > + ip4->sin_family = AF_INET;
> > + ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
> > + ip4->sin_port = hdr->port;
> > + }
> > +}
> > +
> > +static __be16 cm_port_from_service_id(__be64 service_id)
> > +{
> > + return htons(be64_to_cpu(service_id));
> > +}
> > +
> > +static int cm_save_ip_info(struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr,
> > + struct cm_work *work)
> > +{
> > + struct cm_hdr *hdr;
> > + __be16 port;
> > +
> > + hdr = work->cm_event.private_data;
> > + if (hdr->cm_version != RDMA_IP_CM_VERSION)
> > + return -EINVAL;
> > +
> > + port = cm_port_from_service_id(work->path->service_id);
> > +
> > + switch (cm_get_ip_ver(hdr)) {
> > + case 4:
> > + cm_save_ip4_info(src_addr, dst_addr, hdr, port);
> > + break;
> > + case 6:
> > + cm_save_ip6_info(src_addr, dst_addr, hdr, port);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int cm_save_net_info(struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr,
> > + struct ib_cm_event *ib_event)
> > +{
> > + struct cm_work *work = container_of(ib_event, struct cm_work,
> cm_event);
> > +
> > + if ((rdma_port_get_link_layer(work->port->cm_dev->ib_device,
> > + work->port->port_num) ==
> > + IB_LINK_LAYER_INFINIBAND) &&
> > + (ib_event->event == IB_CM_REQ_RECEIVED)) {
> > + cm_save_ib_info(src_addr, dst_addr,
> > + ib_event->param.req_rcvd.primary_path);
> > + return 0;
> > + }
> > +
> > + return cm_save_ip_info(src_addr, dst_addr, work);
> > +}
> > +EXPORT_SYMBOL(cm_save_net_info);
> > +
> > struct ib_cm_id *ib_create_cm_id(struct ib_device *device,
> > ib_cm_handler cm_handler,
> > void *context)
> > @@ -1260,6 +1425,7 @@ static void cm_format_paths_from_req(struct
> cm_req_msg *req_msg,
> > primary_path->packet_life_time =
> > cm_req_get_primary_local_ack_timeout(req_msg);
> > primary_path->packet_life_time -= (primary_path->packet_life_time >
> 0);
> > + primary_path->service_id = req_msg->service_id;
> >
> > if (req_msg->alt_local_lid) {
> > memset(alt_path, 0, sizeof *alt_path);
> > @@ -1281,6 +1447,7 @@ static void cm_format_paths_from_req(struct
> cm_req_msg *req_msg,
> > alt_path->packet_life_time =
> > cm_req_get_alt_local_ack_timeout(req_msg);
> > alt_path->packet_life_time -= (alt_path->packet_life_time >
> 0);
> > + alt_path->service_id = req_msg->service_id;
> > }
> > }
> >
> > diff --git a/drivers/infiniband/core/cma.c
> b/drivers/infiniband/core/cma.c
> > index aeb2417ec928..9f6faeb1de5f 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -179,23 +179,8 @@ struct iboe_mcast_work {
> > struct cma_multicast *mc;
> > };
> >
> > -union cma_ip_addr {
> > - struct in6_addr ip6;
> > - struct {
> > - __be32 pad[3];
> > - __be32 addr;
> > - } ip4;
> > -};
> >
> > -struct cma_hdr {
> > - u8 cma_version;
> > - u8 ip_version; /* IP version: 7:4 */
> > - __be16 port;
> > - union cma_ip_addr src_addr;
> > - union cma_ip_addr dst_addr;
> > -};
> >
> > -#define CMA_VERSION 0x00
> >
> > static int cma_comp(struct rdma_id_private *id_priv, enum
> rdma_cm_state comp)
> > {
> > @@ -234,16 +219,6 @@ static enum rdma_cm_state cma_exch(struct
> rdma_id_private *id_priv,
> > return old;
> > }
> >
> > -static inline u8 cma_get_ip_ver(struct cma_hdr *hdr)
> > -{
> > - return hdr->ip_version >> 4;
> > -}
> > -
> > -static inline void cma_set_ip_ver(struct cma_hdr *hdr, u8 ip_ver)
> > -{
> > - hdr->ip_version = (ip_ver << 4) | (hdr->ip_version & 0xF);
> > -}
> > -
> > static void cma_attach_to_dev(struct rdma_id_private *id_priv,
> > struct cma_device *cma_dev)
> > {
> > @@ -839,93 +814,9 @@ static inline int cma_any_port(struct sockaddr
> *addr)
> > return !cma_port(addr);
> > }
> >
> > -static void cma_save_ib_info(struct rdma_cm_id *id, struct rdma_cm_id
> *listen_id,
> > - struct ib_sa_path_rec *path)
> > -{
> > - struct sockaddr_ib *listen_ib, *ib;
> > -
> > - listen_ib = (struct sockaddr_ib *) &listen_id->route.addr.src_addr;
> > - ib = (struct sockaddr_ib *) &id->route.addr.src_addr;
> > - ib->sib_family = listen_ib->sib_family;
> > - ib->sib_pkey = path->pkey;
> > - ib->sib_flowinfo = path->flow_label;
> > - memcpy(&ib->sib_addr, &path->sgid, 16);
> > - ib->sib_sid = listen_ib->sib_sid;
> > - ib->sib_sid_mask = cpu_to_be64(0xffffffffffffffffULL);
> > - ib->sib_scope_id = listen_ib->sib_scope_id;
> > -
> > - ib = (struct sockaddr_ib *) &id->route.addr.dst_addr;
> > - ib->sib_family = listen_ib->sib_family;
> > - ib->sib_pkey = path->pkey;
> > - ib->sib_flowinfo = path->flow_label;
> > - memcpy(&ib->sib_addr, &path->dgid, 16);
> > -}
> > -
> > -static void cma_save_ip4_info(struct rdma_cm_id *id, struct
> rdma_cm_id *listen_id,
> > - struct cma_hdr *hdr)
> > -{
> > - struct sockaddr_in *listen4, *ip4;
> > -
> > - listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
> > - ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
> > - ip4->sin_family = AF_INET;
> > - ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
> > - ip4->sin_port = listen4->sin_port;
> > -
> > - ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr;
> > - ip4->sin_family = AF_INET;
> > - ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
> > - ip4->sin_port = hdr->port;
> > -}
> > -
> > -static void cma_save_ip6_info(struct rdma_cm_id *id, struct
> rdma_cm_id *listen_id,
> > - struct cma_hdr *hdr)
> > -{
> > - struct sockaddr_in6 *listen6, *ip6;
> > -
> > - listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr;
> > - ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr;
> > - ip6->sin6_family = AF_INET6;
> > - ip6->sin6_addr = hdr->dst_addr.ip6;
> > - ip6->sin6_port = listen6->sin6_port;
> > -
> > - ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr;
> > - ip6->sin6_family = AF_INET6;
> > - ip6->sin6_addr = hdr->src_addr.ip6;
> > - ip6->sin6_port = hdr->port;
> > -}
> > -
> > -static int cma_save_net_info(struct rdma_cm_id *id, struct rdma_cm_id
> *listen_id,
> > - struct ib_cm_event *ib_event)
> > -{
> > - struct cma_hdr *hdr;
> > -
> > - if ((listen_id->route.addr.src_addr.ss_family == AF_IB) &&
> > - (ib_event->event == IB_CM_REQ_RECEIVED)) {
> > - cma_save_ib_info(id, listen_id, ib_event-
> >param.req_rcvd.primary_path);
> > - return 0;
> > - }
> > -
> > - hdr = ib_event->private_data;
> > - if (hdr->cma_version != CMA_VERSION)
> > - return -EINVAL;
> > -
> > - switch (cma_get_ip_ver(hdr)) {
> > - case 4:
> > - cma_save_ip4_info(id, listen_id, hdr);
> > - break;
> > - case 6:
> > - cma_save_ip6_info(id, listen_id, hdr);
> > - break;
> > - default:
> > - return -EINVAL;
> > - }
> > - return 0;
> > -}
> > -
> > static inline int cma_user_data_offset(struct rdma_id_private
> *id_priv)
> > {
> > - return cma_family(id_priv) == AF_IB ? 0 : sizeof(struct cma_hdr);
> > + return cma_family(id_priv) == AF_IB ? 0 : sizeof(struct cm_hdr);
> > }
> >
> > static void cma_cancel_route(struct rdma_id_private *id_priv)
> > @@ -1195,7 +1086,9 @@ static struct rdma_id_private
> *cma_new_conn_id(struct rdma_cm_id *listen_id,
> > return NULL;
> >
> > id_priv = container_of(id, struct rdma_id_private, id);
> > - if (cma_save_net_info(id, listen_id, ib_event))
> > + if (cm_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
> > + (struct sockaddr *)&id->route.addr.dst_addr,
> > + ib_event))
> > goto err;
> >
> > rt = &id->route;
> > @@ -1241,7 +1134,9 @@ static struct rdma_id_private
> *cma_new_udp_id(struct rdma_cm_id *listen_id,
> > return NULL;
> >
> > id_priv = container_of(id, struct rdma_id_private, id);
> > - if (cma_save_net_info(id, listen_id, ib_event))
> > + if (cm_save_net_info((struct sockaddr *)&id->route.addr.src_addr,
> > + (struct sockaddr *)&id->route.addr.dst_addr,
> > + ib_event))
> > goto err;
> >
> > if (!cma_any_addr((struct sockaddr *) &id->route.addr.src_addr)) {
> > @@ -1369,7 +1264,7 @@ EXPORT_SYMBOL(rdma_get_service_id);
> > static void cma_set_compare_data(enum rdma_port_space ps, struct
> sockaddr *addr,
> > struct ib_cm_compare_data *compare)
> > {
> > - struct cma_hdr *cma_data, *cma_mask;
> > + struct cm_hdr *cma_data, *cma_mask;
> > __be32 ip4_addr;
> > struct in6_addr ip6_addr;
> >
> > @@ -1380,8 +1275,8 @@ static void cma_set_compare_data(enum
> rdma_port_space ps, struct sockaddr *addr,
> > switch (addr->sa_family) {
> > case AF_INET:
> > ip4_addr = ((struct sockaddr_in *) addr)->sin_addr.s_addr;
> > - cma_set_ip_ver(cma_data, 4);
> > - cma_set_ip_ver(cma_mask, 0xF);
> > + cm_set_ip_ver(cma_data, 4);
> > + cm_set_ip_ver(cma_mask, 0xF);
> > if (!cma_any_addr(addr)) {
> > cma_data->dst_addr.ip4.addr = ip4_addr;
> > cma_mask->dst_addr.ip4.addr = htonl(~0);
> > @@ -1389,8 +1284,8 @@ static void cma_set_compare_data(enum
> rdma_port_space ps, struct sockaddr *addr,
> > break;
> > case AF_INET6:
> > ip6_addr = ((struct sockaddr_in6 *) addr)->sin6_addr;
> > - cma_set_ip_ver(cma_data, 6);
> > - cma_set_ip_ver(cma_mask, 0xF);
> > + cm_set_ip_ver(cma_data, 6);
> > + cm_set_ip_ver(cma_mask, 0xF);
> > if (!cma_any_addr(addr)) {
> > cma_data->dst_addr.ip6 = ip6_addr;
> > memset(&cma_mask->dst_addr.ip6, 0xFF,
> > @@ -2615,35 +2510,6 @@ err1:
> > }
> > EXPORT_SYMBOL(rdma_bind_addr);
> >
> > -static int cma_format_hdr(void *hdr, struct rdma_id_private *id_priv)
> > -{
> > - struct cma_hdr *cma_hdr;
> > -
> > - cma_hdr = hdr;
> > - cma_hdr->cma_version = CMA_VERSION;
> > - if (cma_family(id_priv) == AF_INET) {
> > - struct sockaddr_in *src4, *dst4;
> > -
> > - src4 = (struct sockaddr_in *) cma_src_addr(id_priv);
> > - dst4 = (struct sockaddr_in *) cma_dst_addr(id_priv);
> > -
> > - cma_set_ip_ver(cma_hdr, 4);
> > - cma_hdr->src_addr.ip4.addr = src4->sin_addr.s_addr;
> > - cma_hdr->dst_addr.ip4.addr = dst4->sin_addr.s_addr;
> > - cma_hdr->port = src4->sin_port;
> > - } else if (cma_family(id_priv) == AF_INET6) {
> > - struct sockaddr_in6 *src6, *dst6;
> > -
> > - src6 = (struct sockaddr_in6 *) cma_src_addr(id_priv);
> > - dst6 = (struct sockaddr_in6 *) cma_dst_addr(id_priv);
> > -
> > - cma_set_ip_ver(cma_hdr, 6);
> > - cma_hdr->src_addr.ip6 = src6->sin6_addr;
> > - cma_hdr->dst_addr.ip6 = dst6->sin6_addr;
> > - cma_hdr->port = src6->sin6_port;
> > - }
> > - return 0;
> > -}
> >
> > static int cma_sidr_rep_handler(struct ib_cm_id *cm_id,
> > struct ib_cm_event *ib_event)
> > @@ -2731,7 +2597,9 @@ static int cma_resolve_ib_udp(struct
> rdma_id_private *id_priv,
> > conn_param->private_data_len);
> >
> > if (private_data) {
> > - ret = cma_format_hdr(private_data, id_priv);
> > + ret = cm_format_hdr(private_data, cma_family(id_priv),
> > + cma_src_addr(id_priv),
> > + cma_dst_addr(id_priv));
> > if (ret)
> > goto out;
> > req.private_data = private_data;
> > @@ -2796,7 +2664,9 @@ static int cma_connect_ib(struct rdma_id_private
> *id_priv,
> >
> > route = &id_priv->id.route;
> > if (private_data) {
> > - ret = cma_format_hdr(private_data, id_priv);
> > + ret = cm_format_hdr(private_data, cma_family(id_priv),
> > + cma_src_addr(id_priv),
> > + cma_dst_addr(id_priv));
> > if (ret)
> > goto out;
> > req.private_data = private_data;
> > diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
> > index 0e3ff30647d5..e418a11afcfe 100644
> > --- a/include/rdma/ib_cm.h
> > +++ b/include/rdma/ib_cm.h
> > @@ -274,6 +274,52 @@ struct ib_cm_event {
> > #define CM_LAP_ATTR_ID cpu_to_be16(0x0019)
> > #define CM_APR_ATTR_ID cpu_to_be16(0x001A)
> >
> > +union cm_ip_addr {
> > + struct in6_addr ip6;
> > + struct {
> > + __be32 pad[3];
> > + __be32 addr;
> > + } ip4;
> > +};
> > +
> > +struct cm_hdr {
> > + u8 cm_version;
> > + u8 ip_version; /* IP version: 7:4 */
> > + __be16 port;
> > + union cm_ip_addr src_addr;
> > + union cm_ip_addr dst_addr;
> > +};
> > +
> > +#define RDMA_IP_CM_VERSION 0x00
> > +
> > +/**
> > + * cm_format_hdr - Fill in a cm_hdr struct according to connection
> details
> > + * @hdr: cm_hdr struct to fill
> > + * @family: ip family of the addresses - AF_INET or AF_INTET6
> > + * @src_addr: source address of the connection
> > + * @dst_addr: destination address of the connection
> > + **/
> > +int cm_format_hdr(void *hdr, int family,
> > + struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr);
> > +
> > +/**
> > + * cm_save_net_info - saves ib connection event details
> > + * @src_addr: source address of the connection
> > + * @dst_addr: destination address of the connection
> > + * @ib_event: ib event to take connection details from
> > + **/
> > +int cm_save_net_info(struct sockaddr *src_addr,
> > + struct sockaddr *dst_addr,
> > + struct ib_cm_event *ib_event);
> > +
> > +/**
> > + * cm_set_ip_ver - sets the ip version of a cm_hdr struct
> > + * @hdr: cm_hdr struct to change
> > + * @ip_ver: ip version to set - a 4 bit value
> > + **/
> > +void cm_set_ip_ver(struct cm_hdr *hdr, u8 ip_ver);
> > +
> > /**
> > * ib_cm_handler - User-defined callback to process communication
> events.
> > * @cm_id: Communication identifier associated with the reported
> event.
>
> Every other symbols in ib_cm.h are prefixed by "ib_cm_", so I would
> prefer having symbols moved from ib_cma.c to ib_cm.c be renamed, except
> it would create a lot of code change ...
>
Sadly, ib_cm does not have such scheme for exported symbols as of time being:
$ git checkout v3.19-rc6
Note: checking out 'v3.19-rc6'
...
HEAD is now at 26bc420b... Linux 3.19-rc6
$ grep EXPORT drivers/infiniband/core/cm.c
EXPORT_SYMBOL(ib_create_cm_id);
EXPORT_SYMBOL(ib_destroy_cm_id);
EXPORT_SYMBOL(ib_cm_listen);
EXPORT_SYMBOL(ib_send_cm_req);
EXPORT_SYMBOL(ib_send_cm_rep);
EXPORT_SYMBOL(ib_send_cm_rtu);
EXPORT_SYMBOL(ib_send_cm_dreq);
EXPORT_SYMBOL(ib_send_cm_drep);
EXPORT_SYMBOL(ib_send_cm_rej);
EXPORT_SYMBOL(ib_send_cm_mra);
EXPORT_SYMBOL(ib_send_cm_lap);
EXPORT_SYMBOL(ib_send_cm_apr);
EXPORT_SYMBOL(ib_send_cm_sidr_req);
EXPORT_SYMBOL(ib_send_cm_sidr_rep);
EXPORT_SYMBOL(ib_cm_notify);
EXPORT_SYMBOL(ib_cm_init_qp_attr);
EXPORT_SYMBOL(cm_class);
I feel that going through changing the names will not be productive, especially considering the inconsistency that already exists in the ib_cm exported symbols names.
On the other hand, patches to fix this naming inconsistency in ib_cm are welcome ;)
Thanks,
--Shachar
^ permalink raw reply
* Re: [PATCH for-next 01/10] IB/addr: Pass network namespace as a parameter
From: Yann Droneaud @ 2015-02-01 14:38 UTC (permalink / raw)
To: Shachar Raindel
Cc: roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Liran Liss,
Guy Shapiro, Haggai Eran, Yotam Kenneth
In-Reply-To: <AM3PR05MB0935B7B53439298A7429158BDC3F0-LOZWmgKjnYgQouBfZGh8ttqRiQSDpxhJvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
Hi,
Le dimanche 01 février 2015 à 13:46 +0000, Shachar Raindel a écrit :
> > -----Original Message-----
> > From: Yann Droneaud [mailto:ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org]
> > Sent: Sunday, February 01, 2015 2:23 PM
> > To: Shachar Raindel
> > Cc: roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org; linux-rdma-u79uwXL29TZUIDd8j+nm9g@public.gmane.org.org;
> > netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Liran Liss; Guy Shapiro; Haggai Eran; Yotam
> > Kenneth
> > Subject: Re: [PATCH for-next 01/10] IB/addr: Pass network namespace as a
> > parameter
> >
> > Hi,
> >
> > Le dimanche 01 février 2015 à 13:28 +0200, Shachar Raindel a écrit :
> > > From: Guy Shapiro <guysh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > > Add network namespace support to the ib_addr module. For that, all the
> > address
> > > resolution and matching should be done using the appropriate namespace
> > instead
> > > of init_net.
> > >
> > > This is achieved by:
> > >
> > > 1. Adding an explicit network namespace argument to exported function
> > that
> > > require a namespace.
> > > 2. Saving the namespace in the rdma_addr_client structure.
> > > 3. Using it when calling networking functions.
> > >
> > > In order to preserve the behavior of calling modules, &init_net is
> > > passed as the parameter in calls from other modules. This is modified
> > as
> > > namspace support is added on more levels.
> >
> > typo: "namespace"
> >
>
> Thanks. Will fix in next iteration.
>
> > >
> > > Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Signed-off-by: Yotam Kenneth <yotamke-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Signed-off-by: Shachar Raindel <raindel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > > Signed-off-by: Guy Shapiro <guysh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > >
> > > ---
> > > drivers/infiniband/core/addr.c | 31 ++++++++++++----------
> > > drivers/infiniband/core/cma.c | 4 ++-
> > > drivers/infiniband/core/verbs.c | 14 +++++++---
> > > drivers/infiniband/hw/ocrdma/ocrdma_ah.c | 3 ++-
> > > include/rdma/ib_addr.h | 44
> > ++++++++++++++++++++++++++++----
> > > 5 files changed, 72 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/addr.c
> > b/drivers/infiniband/core/addr.c
> > > index f80da50d84a5..95beaef6b66d 100644
> > > --- a/drivers/infiniband/core/addr.c
> > > +++ b/drivers/infiniband/core/addr.c
> > > @@ -128,7 +128,7 @@ int rdma_translate_ip(struct sockaddr *addr,
> > struct rdma_dev_addr *dev_addr,
> > > int ret = -EADDRNOTAVAIL;
> > >
> > > if (dev_addr->bound_dev_if) {
> > > - dev = dev_get_by_index(&init_net, dev_addr->bound_dev_if);
> > > + dev = dev_get_by_index(dev_addr->net, dev_addr-
> > >bound_dev_if);
> > > if (!dev)
> > > return -ENODEV;
> > > ret = rdma_copy_addr(dev_addr, dev, NULL);
> > > @@ -137,9 +137,10 @@ int rdma_translate_ip(struct sockaddr *addr,
> > struct rdma_dev_addr *dev_addr,
> > > }
> > >
> > > switch (addr->sa_family) {
> > > - case AF_INET:
> > > - dev = ip_dev_find(&init_net,
> > > - ((struct sockaddr_in *) addr)->sin_addr.s_addr);
> > > + case AF_INET: {
> > > + struct sockaddr_in *addr_in = (struct sockaddr_in *)addr;
> > > +
> > > + dev = ip_dev_find(dev_addr->net, addr_in->sin_addr.s_addr);
> >
> > I don't see the point of this change.
> >
>
> Note that we changed &init_net to be dev_addr->net .
> The rest of the change here was done to avoid issues with checkpatch, as the line was getting really long.
>
> > >
> > > if (!dev)
> > > return ret;
> > > @@ -149,12 +150,12 @@ int rdma_translate_ip(struct sockaddr *addr,
> > struct rdma_dev_addr *dev_addr,
> > > *vlan_id = rdma_vlan_dev_vlan_id(dev);
> > > dev_put(dev);
> > > break;
> > > -
> > > + }
> >
> > closing } here ?
>
> We opened a block in the beginning of this case ("case AF_INET: {"), we close it at the end of the case.
>
> >
> > > #if IS_ENABLED(CONFIG_IPV6)
> > > case AF_INET6:
> > > rcu_read_lock();
> > > - for_each_netdev_rcu(&init_net, dev) {
> > > - if (ipv6_chk_addr(&init_net,
> > > + for_each_netdev_rcu(dev_addr->net, dev) {
> > > + if (ipv6_chk_addr(dev_addr->net,
> > > &((struct sockaddr_in6 *) addr)->sin6_addr,
> > > dev, 1)) {
> > > ret = rdma_copy_addr(dev_addr, dev, NULL);
> > > @@ -236,7 +237,7 @@ static int addr4_resolve(struct sockaddr_in
> > *src_in,
> > > fl4.daddr = dst_ip;
> > > fl4.saddr = src_ip;
> > > fl4.flowi4_oif = addr->bound_dev_if;
> > > - rt = ip_route_output_key(&init_net, &fl4);
> > > + rt = ip_route_output_key(addr->net, &fl4);
> > > if (IS_ERR(rt)) {
> > > ret = PTR_ERR(rt);
> > > goto out;
> > > @@ -278,12 +279,13 @@ static int addr6_resolve(struct sockaddr_in6
> > *src_in,
> > > fl6.saddr = src_in->sin6_addr;
> > > fl6.flowi6_oif = addr->bound_dev_if;
> > >
> > > - dst = ip6_route_output(&init_net, NULL, &fl6);
> > > + dst = ip6_route_output(addr->net, NULL, &fl6);
> > > if ((ret = dst->error))
> > > goto put;
> > >
> > > if (ipv6_addr_any(&fl6.saddr)) {
> > > - ret = ipv6_dev_get_saddr(&init_net, ip6_dst_idev(dst)->dev,
> > > + ret = ipv6_dev_get_saddr(addr->net,
> > > + ip6_dst_idev(dst)->dev,
> > > &fl6.daddr, 0, &fl6.saddr);
> > > if (ret)
> > > goto put;
> > > @@ -458,7 +460,7 @@ static void resolve_cb(int status, struct sockaddr
> > *src_addr,
> > > }
> > >
> > > int rdma_addr_find_dmac_by_grh(union ib_gid *sgid, union ib_gid
> > *dgid, u8 *dmac,
> > > - u16 *vlan_id)
> > > + u16 *vlan_id, struct net *net)
> > > {
> > > int ret = 0;
> > > struct rdma_dev_addr dev_addr;
> > > @@ -481,6 +483,7 @@ int rdma_addr_find_dmac_by_grh(union ib_gid *sgid,
> > union ib_gid *dgid, u8 *dmac,
> > > return ret;
> > >
> > > memset(&dev_addr, 0, sizeof(dev_addr));
> > > + dev_addr.net = net;
> >
> > Should be get_net() be used somewhere to grab a reference on the net
> > namespace ?
> >
>
> Not needed, as dev_addr.net is used only inside this function. Assuming that the caller guarantees that the network namespace doesn't disappear until the function returns, there is no need to take a reference here. This kind of assumption makes sense, as otherwise we will not be able to use the argument at all.
>
> > >
> > > ctx.addr = &dev_addr;
> > > init_completion(&ctx.comp);
> > > @@ -492,7 +495,7 @@ int rdma_addr_find_dmac_by_grh(union ib_gid *sgid,
> > union ib_gid *dgid, u8 *dmac,
> > > wait_for_completion(&ctx.comp);
> > >
> > > memcpy(dmac, dev_addr.dst_dev_addr, ETH_ALEN);
> > > - dev = dev_get_by_index(&init_net, dev_addr.bound_dev_if);
> > > + dev = dev_get_by_index(net, dev_addr.bound_dev_if);
> > > if (!dev)
> > > return -ENODEV;
> > > if (vlan_id)
> > > @@ -502,7 +505,8 @@ int rdma_addr_find_dmac_by_grh(union ib_gid *sgid,
> > union ib_gid *dgid, u8 *dmac,
> > > }
> > > EXPORT_SYMBOL(rdma_addr_find_dmac_by_grh);
> > >
> > > -int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> > *vlan_id)
> > > +int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> > *vlan_id,
> > > + struct net *net)
> > > {
> > > int ret = 0;
> > > struct rdma_dev_addr dev_addr;
> > > @@ -517,6 +521,7 @@ int rdma_addr_find_smac_by_sgid(union ib_gid
> > *sgid, u8 *smac, u16 *vlan_id)
> > > if (ret)
> > > return ret;
> > > memset(&dev_addr, 0, sizeof(dev_addr));
> > > + dev_addr.net = net;
> >
> > get_net() ?
> >
>
> Same as before - used only in the function, caller must make sure it doesn't disappear.
>
> > > ret = rdma_translate_ip(&gid_addr._sockaddr, &dev_addr, vlan_id);
> > > if (ret)
> > > return ret;
> > > diff --git a/drivers/infiniband/core/cma.c
> > b/drivers/infiniband/core/cma.c
> > > index 6e5e11ca7702..aeb2417ec928 100644
> > > --- a/drivers/infiniband/core/cma.c
> > > +++ b/drivers/infiniband/core/cma.c
> > > @@ -512,6 +512,7 @@ struct rdma_cm_id
> > *rdma_create_id(rdma_cm_event_handler event_handler,
> > > INIT_LIST_HEAD(&id_priv->listen_list);
> > > INIT_LIST_HEAD(&id_priv->mc_list);
> > > get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
> > > + id_priv->id.route.addr.dev_addr.net = &init_net;
> > >
> > > return &id_priv->id;
> > > }
> > > @@ -637,7 +638,8 @@ static int cma_modify_qp_rtr(struct
> > rdma_id_private *id_priv,
> > > == RDMA_TRANSPORT_IB &&
> > > rdma_port_get_link_layer(id_priv->id.device, id_priv-
> > >id.port_num)
> > > == IB_LINK_LAYER_ETHERNET) {
> > > - ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL);
> > > + ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr.smac, NULL,
> > > + &init_net);
> > >
> > > if (ret)
> > > goto out;
> > > diff --git a/drivers/infiniband/core/verbs.c
> > b/drivers/infiniband/core/verbs.c
> > > index f93eb8da7b5a..ca5c4dd8a67a 100644
> > > --- a/drivers/infiniband/core/verbs.c
> > > +++ b/drivers/infiniband/core/verbs.c
> > > @@ -212,7 +212,9 @@ int ib_init_ah_from_wc(struct ib_device *device,
> > u8 port_num, struct ib_wc *wc,
> > > ah_attr->vlan_id = wc->vlan_id;
> > > } else {
> > > ret = rdma_addr_find_dmac_by_grh(&grh->dgid, &grh->sgid,
> > > - ah_attr->dmac, &ah_attr->vlan_id);
> > > + ah_attr->dmac,
> > > + &ah_attr->vlan_id,
> > > + &init_net);
> > > if (ret)
> > > return ret;
> > > }
> > > @@ -882,11 +884,15 @@ int ib_resolve_eth_l2_attrs(struct ib_qp *qp,
> > > if (!(*qp_attr_mask & IB_QP_VID))
> > > qp_attr->vlan_id = rdma_get_vlan_id(&sgid);
> > > } else {
> > > - ret = rdma_addr_find_dmac_by_grh(&sgid, &qp_attr-
> > >ah_attr.grh.dgid,
> > > - qp_attr->ah_attr.dmac, &qp_attr->vlan_id);
> > > + ret = rdma_addr_find_dmac_by_grh(
> > > + &sgid,
> > > + &qp_attr->ah_attr.grh.dgid,
> > > + qp_attr->ah_attr.dmac, &qp_attr->vlan_id,
> > > + &init_net);
> > > if (ret)
> > > goto out;
> > > - ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr->smac,
> > NULL);
> > > + ret = rdma_addr_find_smac_by_sgid(&sgid, qp_attr->smac,
> > > + NULL, &init_net);
> > > if (ret)
> > > goto out;
> > > }
> > > diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> > b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> > > index f3cc8c9e65ae..debaac2b6ee8 100644
> > > --- a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> > > +++ b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> > > @@ -119,7 +119,8 @@ struct ib_ah *ocrdma_create_ah(struct ib_pd *ibpd,
> > struct ib_ah_attr *attr)
> > >
> > > if (pd->uctx) {
> > > status = rdma_addr_find_dmac_by_grh(&sgid, &attr->grh.dgid,
> > > - attr->dmac, &attr->vlan_id);
> > > + attr->dmac, &attr->vlan_id,
> > > + &init_net);
> > > if (status) {
> > > pr_err("%s(): Failed to resolve dmac from gid."
> > > "status = %d\n", __func__, status);
> > > diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
> > > index ce55906b54a0..40ccf8b83755 100644
> > > --- a/include/rdma/ib_addr.h
> > > +++ b/include/rdma/ib_addr.h
> > > @@ -47,6 +47,7 @@
> > > #include <rdma/ib_verbs.h>
> > > #include <rdma/ib_pack.h>
> > > #include <net/ipv6.h>
> > > +#include <net/net_namespace.h>
> > >
> > > struct rdma_addr_client {
> > > atomic_t refcount;
> > > @@ -64,6 +65,16 @@ void rdma_addr_register_client(struct
> > rdma_addr_client *client);
> > > */
> > > void rdma_addr_unregister_client(struct rdma_addr_client *client);
> > >
> > > +/**
> > > + * struct rdma_dev_addr - Contains resolved RDMA hardware addresses
> > > + * @src_dev_addr: Source MAC address.
> > > + * @dst_dev_addr: Destination MAC address.
> > > + * @broadcast: Broadcast address of the device.
> > > + * @dev_type: The interface hardware type of the device.
> > > + * @bound_dev_if: An optional device interface index.
> > > + * @transport: The transport type used.
> > > + * @net: Network namespace containing the bound_dev_if
> > net_dev.
> > > + */
> > > struct rdma_dev_addr {
> > > unsigned char src_dev_addr[MAX_ADDR_LEN];
> > > unsigned char dst_dev_addr[MAX_ADDR_LEN];
> > > @@ -71,11 +82,14 @@ struct rdma_dev_addr {
> > > unsigned short dev_type;
> > > int bound_dev_if;
> > > enum rdma_transport_type transport;
> > > + struct net *net;
> > > };
> > >
> > > /**
> > > * rdma_translate_ip - Translate a local IP address to an RDMA
> > hardware
> > > * address.
> > > + *
> > > + * The dev_addr->net field must be initialized.
> > > */
> > > int rdma_translate_ip(struct sockaddr *addr, struct rdma_dev_addr
> > *dev_addr,
> > > u16 *vlan_id);
> > > @@ -90,7 +104,7 @@ int rdma_translate_ip(struct sockaddr *addr, struct
> > rdma_dev_addr *dev_addr,
> > > * @dst_addr: The destination address to resolve.
> > > * @addr: A reference to a data location that will receive the
> > resolved
> > > * addresses. The data location must remain valid until the
> > callback has
> > > - * been invoked.
> > > + * been invoked. The net field of the addr struct must be valid.
> > > * @timeout_ms: Amount of time to wait for the address resolution to
> > complete.
> > > * @callback: Call invoked once address resolution has completed,
> > timed out,
> > > * or been canceled. A status of 0 indicates success.
> > > @@ -110,9 +124,29 @@ int rdma_copy_addr(struct rdma_dev_addr
> > *dev_addr, struct net_device *dev,
> > >
> > > int rdma_addr_size(struct sockaddr *addr);
> > >
> > > -int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> > *vlan_id);
> > > -int rdma_addr_find_dmac_by_grh(union ib_gid *sgid, union ib_gid
> > *dgid, u8 *smac,
> > > - u16 *vlan_id);
> > > +/** rdma_addr_find_smac_by_sgid() - Find the src MAC and VLAN ID for
> > a src GID
> > > + * @sgid: Source GID to find the MAC and VLAN for.
> > > + * @smac: A buffer to contain the resulting MAC address.
> > > + * @vlan_id: Will contain the resulting VLAN ID.
> > > + * @net: Network namespace to use for the address resolution.
> > > + *
> > > + * It is the caller's responsibility to keep the network namespace
> > alive until
> > > + * the function returns.
> >
> > Why ?
> >
>
> So that we could use the argument. Otherwise, we will need to have ugly code like:
> ------------------------
> struct net *local_net = NULL;
> rcu_read_lock();
> for_each_net_rcu(local_net)
> if (local_net == net)
> break;
> if (local_net == net)
> get_net(local_net);
> else
> local_net = NULL;
> rcu_read_unlock();
> ------------------------
> however, the callers (in following patches), can easily ensure that the network namespace is here to stay. This is much easier to understand and maintain.
>
>
> > > + */
> > > +int rdma_addr_find_smac_by_sgid(union ib_gid *sgid, u8 *smac, u16
> > *vlan_id,
> > > + struct net *net);
> > > +/** rdma_addr_find_dmac_by_grh() - Find the dst MAC and VLAN ID for a
> > GID pair
> > > + * @sgid: Source GID to use for the search.
> > > + * @dgid: Destination GID to find the details for.
> > > + * @dmac: Contains the resulting destination MAC address.
> > > + * @vlan_id: Contains the resulting VLAN ID.
> > > + * @net: Network namespace to use for the address resolution.
> > > + *
> > > + * It is the caller's responsibility to keep the network namespace
> > alive until
> > > + * the function returns.
> >
> > Why ?
> >
>
> See above.
>
> >
> >
> > I believe this patch lack proper reference counting in form of
> > get_net() / put_net(), but cannot say for sure.
> >
>
> If you could point to specific issues or race conditions, that would be great.
> We have thoroughly tested and reviewed the code, and couldn't find any such issues with the submitted patches.
>
OK, that's enough for me: I was just concerned about the various
references that were copied to data structure inside the functions
and the functions were asking for the caller to manage the reference
for them, which seems wrong.
So I think the messages "It is the caller's responsibility to keep the
network namespace alive until the function returns" are obvious,
and could be removed.
Regards.
--
Yann Droneaud
OPTEYA
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/3] netlabel: Less function calls in netlbl_mgmt_add_common() after error detection
From: Paul Moore @ 2015-02-01 14:39 UTC (permalink / raw)
To: SF Markus Elfring
Cc: David S. Miller, netdev, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CDFCDA.9010802@users.sourceforge.net>
On Sun, Feb 1, 2015 at 5:15 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 1 Feb 2015 11:11:29 +0100
>
> The functions "cipso_v4_doi_putdef" and "kfree" could be called in some cases
> by the netlbl_mgmt_add_common() function during error handling even if the
> passed variables contained still a null pointer.
>
> * This implementation detail could be improved by adjustments for jump labels.
>
> * Let us return immediately after the first failed function call according to
> the current Linux coding style convention.
>
> * Let us delete also an unnecessary check for the variable "entry" there.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/netlabel/netlabel_mgmt.c | 49 ++++++++++++++++++++++----------------------
> 1 file changed, 24 insertions(+), 25 deletions(-)
Thanks for fixing the label names.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> index f5807f5..7044074 100644
> --- a/net/netlabel/netlabel_mgmt.c
> +++ b/net/netlabel/netlabel_mgmt.c
> @@ -93,23 +93,20 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> struct netlbl_audit *audit_info)
> {
> int ret_val = -EINVAL;
> - struct netlbl_dom_map *entry = NULL;
> struct netlbl_domaddr_map *addrmap = NULL;
> struct cipso_v4_doi *cipsov4 = NULL;
> u32 tmp_val;
> + struct netlbl_dom_map *entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>
> - entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> - if (entry == NULL) {
> - ret_val = -ENOMEM;
> - goto add_failure;
> - }
> + if (!entry)
> + return -ENOMEM;
> entry->def.type = nla_get_u32(info->attrs[NLBL_MGMT_A_PROTOCOL]);
> if (info->attrs[NLBL_MGMT_A_DOMAIN]) {
> size_t tmp_size = nla_len(info->attrs[NLBL_MGMT_A_DOMAIN]);
> entry->domain = kmalloc(tmp_size, GFP_KERNEL);
> if (entry->domain == NULL) {
> ret_val = -ENOMEM;
> - goto add_failure;
> + goto add_free_entry;
> }
> nla_strlcpy(entry->domain,
> info->attrs[NLBL_MGMT_A_DOMAIN], tmp_size);
> @@ -125,16 +122,16 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> break;
> case NETLBL_NLTYPE_CIPSOV4:
> if (!info->attrs[NLBL_MGMT_A_CV4DOI])
> - goto add_failure;
> + goto add_free_domain;
>
> tmp_val = nla_get_u32(info->attrs[NLBL_MGMT_A_CV4DOI]);
> cipsov4 = cipso_v4_doi_getdef(tmp_val);
> if (cipsov4 == NULL)
> - goto add_failure;
> + goto add_free_domain;
> entry->def.cipso = cipsov4;
> break;
> default:
> - goto add_failure;
> + goto add_free_domain;
> }
>
> if (info->attrs[NLBL_MGMT_A_IPV4ADDR]) {
> @@ -145,7 +142,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> addrmap = kzalloc(sizeof(*addrmap), GFP_KERNEL);
> if (addrmap == NULL) {
> ret_val = -ENOMEM;
> - goto add_failure;
> + goto add_doi_put_def;
> }
> INIT_LIST_HEAD(&addrmap->list4);
> INIT_LIST_HEAD(&addrmap->list6);
> @@ -153,12 +150,12 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> if (nla_len(info->attrs[NLBL_MGMT_A_IPV4ADDR]) !=
> sizeof(struct in_addr)) {
> ret_val = -EINVAL;
> - goto add_failure;
> + goto add_free_addrmap;
> }
> if (nla_len(info->attrs[NLBL_MGMT_A_IPV4MASK]) !=
> sizeof(struct in_addr)) {
> ret_val = -EINVAL;
> - goto add_failure;
> + goto add_free_addrmap;
> }
> addr = nla_data(info->attrs[NLBL_MGMT_A_IPV4ADDR]);
> mask = nla_data(info->attrs[NLBL_MGMT_A_IPV4MASK]);
> @@ -166,7 +163,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> map = kzalloc(sizeof(*map), GFP_KERNEL);
> if (map == NULL) {
> ret_val = -ENOMEM;
> - goto add_failure;
> + goto add_free_addrmap;
> }
> map->list.addr = addr->s_addr & mask->s_addr;
> map->list.mask = mask->s_addr;
> @@ -178,7 +175,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> ret_val = netlbl_af4list_add(&map->list, &addrmap->list4);
> if (ret_val != 0) {
> kfree(map);
> - goto add_failure;
> + goto add_free_addrmap;
> }
>
> entry->def.type = NETLBL_NLTYPE_ADDRSELECT;
> @@ -192,7 +189,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> addrmap = kzalloc(sizeof(*addrmap), GFP_KERNEL);
> if (addrmap == NULL) {
> ret_val = -ENOMEM;
> - goto add_failure;
> + goto add_doi_put_def;
> }
> INIT_LIST_HEAD(&addrmap->list4);
> INIT_LIST_HEAD(&addrmap->list6);
> @@ -200,12 +197,12 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> if (nla_len(info->attrs[NLBL_MGMT_A_IPV6ADDR]) !=
> sizeof(struct in6_addr)) {
> ret_val = -EINVAL;
> - goto add_failure;
> + goto add_free_addrmap;
> }
> if (nla_len(info->attrs[NLBL_MGMT_A_IPV6MASK]) !=
> sizeof(struct in6_addr)) {
> ret_val = -EINVAL;
> - goto add_failure;
> + goto add_free_addrmap;
> }
> addr = nla_data(info->attrs[NLBL_MGMT_A_IPV6ADDR]);
> mask = nla_data(info->attrs[NLBL_MGMT_A_IPV6MASK]);
> @@ -213,7 +210,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> map = kzalloc(sizeof(*map), GFP_KERNEL);
> if (map == NULL) {
> ret_val = -ENOMEM;
> - goto add_failure;
> + goto add_free_addrmap;
> }
> map->list.addr = *addr;
> map->list.addr.s6_addr32[0] &= mask->s6_addr32[0];
> @@ -227,7 +224,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> ret_val = netlbl_af6list_add(&map->list, &addrmap->list6);
> if (ret_val != 0) {
> kfree(map);
> - goto add_failure;
> + goto add_free_addrmap;
> }
>
> entry->def.type = NETLBL_NLTYPE_ADDRSELECT;
> @@ -237,15 +234,17 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
>
> ret_val = netlbl_domhsh_add(entry, audit_info);
> if (ret_val != 0)
> - goto add_failure;
> + goto add_free_addrmap;
>
> return 0;
>
> -add_failure:
> - cipso_v4_doi_putdef(cipsov4);
> - if (entry)
> - kfree(entry->domain);
> +add_free_addrmap:
> kfree(addrmap);
> +add_doi_put_def:
> + cipso_v4_doi_putdef(cipsov4);
> +add_free_domain:
> + kfree(entry->domain);
> +add_free_entry:
> kfree(entry);
> return ret_val;
> }
> --
> 2.2.2
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Fwd: Throughput regression with `tcp: refine TSO autosizing`
From: dpreed @ 2015-02-01 14:43 UTC (permalink / raw)
To: Jonathan Morton
Cc: dstanley, Andrew McGregor, Stig Thormodsrud, netdev,
linux-wireless, Jesper Dangaard Brouer, cerowrt-devel,
Matt Mathis, Derrick Pallas, Mahesh Paolini-Subramanya,
Kathy Giori, Tim Shepard, Avery Pennarun
In-Reply-To: <CAJq5cE3ETbpEtecFHmhgHQveNRUhVfrzjTOygHm-TRCTwHNyKA@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 7456 bytes --]
Just to clarify, managing queueing in a single access point WiFi network is only a small part of the problem of fixing the rapidly degrading performance of WiFi based systems. Similarly, mesh routing is only a small part of the problem with the scalability of cooperative meshes based on the WiFi MAC.
So I don't disagree with work on queue management (which is not good in any commercial product).
But Dave T has done some great talks on "fixing WiFi" that don't have to do with queueing very much at all.
For example, rate selection for various packets is terrible. When you have nearly 1000:1 ratios of transmission rates and codes that are not backward compatible, there's a huge opportunity for improvement. Similarly, choice of frequency bandwidth and center frequency at each station offers huge opportunities for practical scalability of systems. Also, as we noted earlier, "handoff" from one next hop to another is a huge problem with performance in practical deployments (a factor of 10x at least, just in that).
Propagation information is not used at all when 802.11 systems share a channel, even in single AP deployments, yet all stations can measure propagation quite accurately in their hardware.
Finally, Listen-before-talk is highly wasteful for two reasons: 1) any random radio noise from other sources unnecessarily degrades communications (and in the 5.8 MHz band, the rule about "radar avoidance" requires treating very low level noise as a "signal to shut the net down by law", but there is a loophole if you can tell that it's not actually "radar" (the technique requires two or more stations to measure the same noise event, and if the power is significantly different - more than a few dB - then it can't possibly be due to a distant transmitter, and therefore can be ignored). 2) the transmitter cannot tell when the intended receiver will be perfectly able to decode the signal without interference with the station it hears (this second point is actually proven in theory in a paper by Jon Peha that argued against trivial "etiquettes" as a mechanism for sharing among uncooperative and non-interoperable stations).
Dave T has discussed more, as have I in other venues.
The reason no one is making progress on any of these particular issues is that there is no coordination at the "systems level" around creating rising tides that lift all boats in the WiFi-ish space. It's all about ripping the competition by creating stuff that can sell better than the other guys' stuff, and avoiding cooperation at all costs.
I agree that, to the extent that managing queues in a single box or a single operating system doesn't require cooperation, it's much easier to get such things into the market. That's why CeroWRT has been as effective as it has been. But has Microsoft done anything at all about it? Do the better ECN signals that can arise from good queue management get used by the TCP endpoints, or for that matter UDP-based protocol endpoints?
But the big wins in making WiFi better are going begging. As WiFi becomes more closed, as it will as the major Internet Access Providers and Gadget builders (Google, Apple) start excluding innovators in wireless from the market by closed, proprietary solutions, the problem WILL get worse. You won't be able to fix those problems at all. If you have a solution you will have to convince the oligopoly to even bother trying it.
So, let me reiterate. The problem is not just "getting Minstrel adopted", though I have nothing against that as a subgoal. The problem is to find good systems-level answers, and to find a strategy to deliver those answers to a WiFi ecology that spans the planet, and where the marketing value-story focuses on things one can measure between two stations in a Faraday cage, and never on any systems-level issues.
I personally think that things like promoting semi-closed, essentially proprietary ESSID-based bridged distribution systems as "good ideas" are counterproductive to this goal. But that's perhaps too radical for this crowd. It reminds me of Cisco's attempt to create a proprietary Internet technology with IOS, which fortunately was not the success Cisco hoped for, or Juniper would not have existed. Maybe IOS would have been a fine standard, but it would have killed the evolution of the Internet as we know it.
On Sunday, February 1, 2015 5:47am, "Jonathan Morton" <chromatix99@gmail.com> said:
Since this is going to be a big job, it's worth prioritising parts of it appropriately.
Minstrel is probably already the single best feature of the Linux Wi-Fi stack. AFAIK it still outperforms any other rate selector we know about. So I don't consider improving it further to be a high priority, although that trick of using it as a sneaky random packet loss inducer is intriguing.
Much more important and urgent is getting some form of functioning SQM closer to the hardware, where the information is. I don't think we need to get super fancy here to do some real good, in the same way that PIE is a major improvement over drop-tail. I'd settle for a variant of fq_codel that gets and uses information about whether the current packet request might be aggregated with the previous packet provided, and adjusts its choice of packet accordingly.
At the same time, models would undoubtedly be useful to help test and repeatably demonstrate the advantages of both simple and more sophisticated solutions. Ns3 allows laying out a reasonably complex radio environment, which is great for this. To counter the prevalence of one-station Faraday cage tests in the industry, the simulated environments should represent realistic, challenging use cases:
1) the family home, with half a dozen client devices competing with several interference sources (Bluetooth, RC toys, microwave oven, etc). This is a relatively easy environment, representing the expected environment for consumer equipment.
2) the apartment block, with fewer clients per AP but lots of APs distributed throughout a large building. Walls and floors may provide valuable attenuation here - unless you're in Japan, where they can be notoriously thin.
3) the railway carriage, consisting of eighty passengers in a 20x3 m space, and roughly the same number of client devices. The uplink is 3G based and has some inherent latency. Add some Bluetooth for flavour, stir gently. This one is rather challenging, but there is scope to optimise AP antenna placement, and to scale the test down slightly by reducing seat occupancy.
4) the jumbo jet, consisting of several hundred passengers crammed in like sardines. The uplink has satellite latencies built in. Good luck.
5) the business hotel. Multiple APs will be needed to provide adequate coverage for this environment, which should encompass the rooms as well as lounge, conference and dining areas. Some visitors may bring their own APs, and the system must be able to cope with this without seriously degrading performance.
6) the trade conference. A large arena filled with thousands of people. Multiple APs required. Good luck.
I also feel that ultimately we're going to have to get industry on board. Not just passively letting us play around as with ath9k, but actively taking note of our findings and implementing at least a few of our ideas themselves. Of course, tools, models and real-world results are likely to make that easier.
- Jonathan Morton
[-- Attachment #1.2: Type: text/html, Size: 11383 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] crypto: algif - change algif_skcipher to be asynchronous
From: Stephan Mueller @ 2015-02-01 18:31 UTC (permalink / raw)
To: Tadeusz Struk
Cc: herbert, linux-crypto, netdev, davem, qat-linux, linux-kernel
In-Reply-To: <20150129231338.25156.65450.stgit@tstruk-mobl1>
Am Donnerstag, 29. Januar 2015, 15:13:39 schrieb Tadeusz Struk:
Hi Tadeusz,
> The way the algif_skcipher works currently is that on sendmsg/sendpage it
> builds an sgl for the input data and then on read/recvmsg it sends the job
> for encryption putting the user to sleep till the data is processed.
> This way it can only handle one job at a given time.
> To be able to fuly utilize the potential of existing crypto hardware
> accelerators it is required to submit multiple jobs in asynchronously.
> First patch enables asynchronous read and write on socket.
> Second patch enables af_alg sgl to be linked.
> Third patch implement asynch read for skcipher.
Do you have a code fragment on how to test that patch? I would like to see
whether I can test that with my libkcapi.
>
> Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
> ---
> Tadeusz Struk (3):
> net: socket: enable async read and write
> crypto: af_alg - Allow to link sgl
> crypto: algif - change algif_skcipher to be asynchronous
>
>
> crypto/af_alg.c | 16 ++
> crypto/algif_skcipher.c | 315
> ++++++++++++++++++++++++++++++++++++++++++++++- include/crypto/if_alg.h |
> 4 -
> include/net/sock.h | 2
> net/socket.c | 48 ++++++-
> 5 files changed, 364 insertions(+), 21 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH] fcntl.h: Fix a typo
From: David Miller @ 2015-02-01 20:15 UTC (permalink / raw)
To: bart.vanassche; +Cc: sfr, netdev
In-Reply-To: <54CE16F9.7030706@sandisk.com>
From: Bart Van Assche <bart.vanassche@sandisk.com>
Date: Sun, 01 Feb 2015 13:07:21 +0100
> In the source file fs/fcntl.c and also in the fcntl() man page one
> can see that the FD_CLOEXEC flag can be manipulated via F_GETFD
> and F_SETFD. Update the comment in <fcntl.h> accordingly.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Netdev is not an appropriate mailing list for this patch, is it?
^ permalink raw reply
* Re: [PATCH] tun: orphan an skb on tx
From: David Miller @ 2015-02-01 20:19 UTC (permalink / raw)
To: dwmw2
Cc: mst, herbert, eric.dumazet, jan.kiszka, paul.moore, netdev,
linux-kernel, qemu-devel
In-Reply-To: <1422797630.11044.32.camel@infradead.org>
From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 01 Feb 2015 13:33:50 +0000
> Of course, now I'm looking closely at the path these packets take to
> leave the box, it starts to offend me that they're being passed up to
> userspace just to encrypt them (as DTLS or ESP) and then send them back
> down to the kernel on a UDP socket. The kernel already knows how to
> {en,de}crypt ESP, and do the sequence number checking on incoming
> packets.
It's funny, I thought we had an IPSEC stack....
^ permalink raw reply
* Re: [PATCH v2 3/3] netlabel: Less function calls in netlbl_mgmt_add_common() after error detection
From: David Miller @ 2015-02-01 20:30 UTC (permalink / raw)
To: elfring; +Cc: paul, netdev, linux-kernel, kernel-janitors, julia.lawall
In-Reply-To: <54CDFCDA.9010802@users.sourceforge.net>
When you post a new version of a patch within a series, you must repost
the entire series, not just the patch which is changing.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 00/11] hso: fix some problems in the disconnect path
From: David Miller @ 2015-02-01 20:33 UTC (permalink / raw)
To: olivier-Ui3EtX6WB9GzQB+pC5nmwQ
Cc: j.dumon-x9gZzRpC1QbQT0dZR+AlfA, dcbw-H+wXaHxf7aLQT0dZR+AlfA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1422620523-15021-1-git-send-email-olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
From: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
Date: Fri, 30 Jan 2015 13:21:52 +0100
> These patches attempt to fix some problems I observed when the hso
> device is disconnected.
> Several patches of this serie are fixing crashes or memleaks when a
> hso device is disconnected.
> This serie of patches is based on v3.18.
>
> changes in v2:
> - Last patch of the serie dropped since another patch fix the issue.
> See http://marc.info/?l=linux-usb&m=142186699418489 for more info.
>
> - Added an extra patch avoiding name conflicts for the rfkill interface.
Series applied to net-next, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [patch] isdn: off by one in connect_res()
From: Dan Carpenter @ 2015-02-01 20:54 UTC (permalink / raw)
To: Armin Schindler; +Cc: Karsten Keil, netdev, kernel-janitors
The bug here is that we use "Reject" as the index into the cau_t[] array
in the else path. Since the cau_t[] has 9 elements if Reject == 9 then
we are reading beyond the end of the array.
My understanding of the code is that it's saying that if Reject is 1 or
too high then that's invalid and we should hang up.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Please review this one a bit carefully. I think it's correct, but I'm
not terribly familiar with this code.
diff --git a/drivers/isdn/hardware/eicon/message.c b/drivers/isdn/hardware/eicon/message.c
index 0b38060..d7c2866 100644
--- a/drivers/isdn/hardware/eicon/message.c
+++ b/drivers/isdn/hardware/eicon/message.c
@@ -1474,7 +1474,7 @@ static byte connect_res(dword Id, word Number, DIVA_CAPI_ADAPTER *a,
add_ai(plci, &parms[5]);
sig_req(plci, REJECT, 0);
}
- else if (Reject == 1 || Reject > 9)
+ else if (Reject == 1 || Reject >= 9)
{
add_ai(plci, &parms[5]);
sig_req(plci, HANGUP, 0);
^ permalink raw reply related
* Re: [PATCH] tun: orphan an skb on tx
From: David Woodhouse @ 2015-02-01 21:29 UTC (permalink / raw)
To: David Miller
Cc: mst, herbert, eric.dumazet, jan.kiszka, netdev, linux-kernel,
qemu-devel
In-Reply-To: <20150201.121948.998046471405758397.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]
On Sun, 2015-02-01 at 12:19 -0800, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Sun, 01 Feb 2015 13:33:50 +0000
>
> > Of course, now I'm looking closely at the path these packets take to
> > leave the box, it starts to offend me that they're being passed up to
> > userspace just to encrypt them (as DTLS or ESP) and then send them back
> > down to the kernel on a UDP socket. The kernel already knows how to
> > {en,de}crypt ESP, and do the sequence number checking on incoming
> > packets.
>
> It's funny, I thought we had an IPSEC stack....
Right. But I'm trying to work out how we can sanely *use* that from a
VPN client.
The client normally sets up a tun device, configuring it with
appropriate IP addresses and routes by invoking vpnc-script or passing
the information back to NetworkManager. The client itself might not even
have root privs, in the NetworkManager case.
The initial authentication and connection are done over HTTPS, and
packets *can* be passed that way if they need to be. But obviously the
client *also* tries to set up a UDP data transport too — which is DTLS
in the case of Cisco AnyConnect, and ESP in UDP for Juniper.
If it *can* get communication over UDP, it'll use it. Otherwise it just
passes packets over the TCP connection. So it needs to dynamically set
up and tear down the ESP/DTLS tunnels as and when they are working.
Ideally we want it such that that packets routed to the tun device get
transparently encrypted and sent out on the UDP socket, and packets
received from UDP and successfully decrypted will appear to have arrived
on the tun device. The user may be manually tweaking the routing, or
setting up firewall/NAT/etc. on the tun device.
I can see how to set up an ESP in UDP tunnel such that it looks like the
packets are actually departing on the *physical* interface (which in
practice I suppose they are). But that's going to be fairly complex to
set up, and extremely non-intuitive and hard to manage for the user. To
the extent that I don't think it's actually deployable.
I really was looking for some way to push down something like an XFRM
state into the tun device and just say "shove them out here until I tell
you otherwise".
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]
^ permalink raw reply
* [PATCH v3] net/fsl_pq_mdio: Document supported compatibles
From: Emil Medve @ 2015-02-01 21:58 UTC (permalink / raw)
To: scottwood, linuxppc-dev, devicetree, netdev; +Cc: Shruti Kanetkar, Emil Medve
From: Shruti Kanetkar <Kanetkar.Shruti@gmail.com>
The device tree binding(s) document has fallen out of sync with the
driver code. Update the list of supported devices to reflect current
driver capabilities
Change-Id: I440d8de2ee2d9c3b7b23e69b3da851cab18a4c9a
Signed-off-by: Shruti Kanetkar <Kanetkar.Shruti@gmail.com>
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---
v3: Separated from the FMan MDIO dt/binding patchset http://patchwork.ozlabs.org/patch/370870
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index be6ea896..1e97532 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -8,7 +8,16 @@ of how to define a PHY.
Required properties:
- reg : Offset and length of the register set for the device
- compatible : Should define the compatible device type for the
- mdio. Currently, this is most likely to be "fsl,gianfar-mdio"
+ mdio. Currently supported strings/devices are:
+ - "fsl,gianfar-tbi"
+ - "fsl,gianfar-mdio"
+ - "fsl,etsec2-tbi"
+ - "fsl,etsec2-mdio"
+ - "fsl,ucc-mdio"
+ - "fsl,fman-mdio"
+ When device_type is "mdio", the following strings are also considered:
+ - "gianfar"
+ - "ucc_geth_phy"
Example:
--
2.2.2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply related
* Re: Fwd: Throughput regression with `tcp: refine TSO autosizing`
From: Andrew McGregor @ 2015-02-01 23:34 UTC (permalink / raw)
To: dpreed
Cc: dstanley, Stig Thormodsrud, netdev, linux-wireless,
Jesper Dangaard Brouer, cerowrt-devel, Matt Mathis,
Derrick Pallas, Kathy Giori, Mahesh Paolini-Subramanya,
Jonathan Morton, Tim Shepard, Avery Pennarun
In-Reply-To: <1422801814.796219699@apps.rackspace.com>
[-- Attachment #1.1: Type: text/plain, Size: 9023 bytes --]
So far as I'm concerned, Minstrel is as widely adopted as we could want; it
could go further, but it doesn't need any help with that. It could be
better, however, and that's a fairly straightforward change. One of the
problems with rate selection is that the standard itself has a bit to say
on what rates to use for what packets, and what it says is braindead. So
if you want to be standards compliant you are also leaving quite a bit of
potential performance on the table.
I missed one item in my list of potential improvements: the most braindead
thing 802.11 has to say about rates is that broadcast and multicast packets
should be sent at 'the lowest basic rate in the current supported rate
set', which is really wasteful. There are a couple of ways of dealing with
this: one, ignore the standard and pick the rate that is most likely to get
the frame to as many neighbours as possible (by a scan of the Minstrel
tables). Or two, fan it out as unicast, which might well take less airtime
(due to aggregation) as well as being much more likely to be delivered,
since you get ACKs and retries by doing that.
As for the industry... sure, there are those forces, but then again there's
a non-trivial amount of influence in this audience.
On Mon, Feb 2, 2015 at 1:43 AM, <dpreed@reed.com> wrote:
> Just to clarify, managing queueing in a single access point WiFi network
> is only a small part of the problem of fixing the rapidly degrading
> performance of WiFi based systems. Similarly, mesh routing is only a small
> part of the problem with the scalability of cooperative meshes based on the
> WiFi MAC.
>
>
>
> So I don't disagree with work on queue management (which is not good in
> any commercial product).
>
>
>
> But Dave T has done some great talks on "fixing WiFi" that don't have to
> do with queueing very much at all.
>
>
>
> For example, rate selection for various packets is terrible. When you
> have nearly 1000:1 ratios of transmission rates and codes that are not
> backward compatible, there's a huge opportunity for improvement.
> Similarly, choice of frequency bandwidth and center frequency at each
> station offers huge opportunities for practical scalability of systems.
> Also, as we noted earlier, "handoff" from one next hop to another is a huge
> problem with performance in practical deployments (a factor of 10x at
> least, just in that).
>
>
>
> Propagation information is not used at all when 802.11 systems share a
> channel, even in single AP deployments, yet all stations can measure
> propagation quite accurately in their hardware.
>
>
>
> Finally, Listen-before-talk is highly wasteful for two reasons: 1) any
> random radio noise from other sources unnecessarily degrades communications
> (and in the 5.8 MHz band, the rule about "radar avoidance" requires
> treating very low level noise as a "signal to shut the net down by law",
> but there is a loophole if you can tell that it's not actually "radar" (the
> technique requires two or more stations to measure the same noise event,
> and if the power is significantly different - more than a few dB - then it
> can't possibly be due to a distant transmitter, and therefore can be
> ignored). 2) the transmitter cannot tell when the intended receiver will be
> perfectly able to decode the signal without interference with the station
> it hears (this second point is actually proven in theory in a paper by Jon
> Peha that argued against trivial "etiquettes" as a mechanism for sharing
> among uncooperative and non-interoperable stations).
>
>
>
> Dave T has discussed more, as have I in other venues.
>
>
>
> The reason no one is making progress on any of these particular issues is
> that there is no coordination at the "systems level" around creating rising
> tides that lift all boats in the WiFi-ish space. It's all about ripping
> the competition by creating stuff that can sell better than the other guys'
> stuff, and avoiding cooperation at all costs.
>
>
>
> I agree that, to the extent that managing queues in a single box or a
> single operating system doesn't require cooperation, it's much easier to
> get such things into the market. That's why CeroWRT has been as effective
> as it has been. But has Microsoft done anything at all about it? Do the
> better ECN signals that can arise from good queue management get used by
> the TCP endpoints, or for that matter UDP-based protocol endpoints?
>
>
>
> But the big wins in making WiFi better are going begging. As WiFi becomes
> more closed, as it will as the major Internet Access Providers and Gadget
> builders (Google, Apple) start excluding innovators in wireless from the
> market by closed, proprietary solutions, the problem WILL get worse. You
> won't be able to fix those problems at all. If you have a solution you
> will have to convince the oligopoly to even bother trying it.
>
>
>
> So, let me reiterate. The problem is not just "getting Minstrel adopted",
> though I have nothing against that as a subgoal. The problem is to find
> good systems-level answers, and to find a strategy to deliver those answers
> to a WiFi ecology that spans the planet, and where the marketing
> value-story focuses on things one can measure between two stations in a
> Faraday cage, and never on any systems-level issues.
>
>
>
>
>
> I personally think that things like promoting semi-closed, essentially
> proprietary ESSID-based bridged distribution systems as "good ideas" are
> counterproductive to this goal. But that's perhaps too radical for this
> crowd. It reminds me of Cisco's attempt to create a proprietary Internet
> technology with IOS, which fortunately was not the success Cisco hoped for,
> or Juniper would not have existed. Maybe IOS would have been a fine
> standard, but it would have killed the evolution of the Internet as we know
> it.
>
>
> On Sunday, February 1, 2015 5:47am, "Jonathan Morton" <
> chromatix99@gmail.com> said:
>
> Since this is going to be a big job, it's worth prioritising parts of it
> appropriately.
>
> Minstrel is probably already the single best feature of the Linux Wi-Fi
> stack. AFAIK it still outperforms any other rate selector we know about. So
> I don't consider improving it further to be a high priority, although that
> trick of using it as a sneaky random packet loss inducer is intriguing.
>
> Much more important and urgent is getting some form of functioning SQM
> closer to the hardware, where the information is. I don't think we need to
> get super fancy here to do some real good, in the same way that PIE is a
> major improvement over drop-tail. I'd settle for a variant of fq_codel that
> gets and uses information about whether the current packet request might be
> aggregated with the previous packet provided, and adjusts its choice of
> packet accordingly.
>
> At the same time, models would undoubtedly be useful to help test and
> repeatably demonstrate the advantages of both simple and more sophisticated
> solutions. Ns3 allows laying out a reasonably complex radio environment,
> which is great for this. To counter the prevalence of one-station Faraday
> cage tests in the industry, the simulated environments should represent
> realistic, challenging use cases:
>
> 1) the family home, with half a dozen client devices competing with
> several interference sources (Bluetooth, RC toys, microwave oven, etc).
> This is a relatively easy environment, representing the expected
> environment for consumer equipment.
>
> 2) the apartment block, with fewer clients per AP but lots of APs
> distributed throughout a large building. Walls and floors may provide
> valuable attenuation here - unless you're in Japan, where they can be
> notoriously thin.
>
> 3) the railway carriage, consisting of eighty passengers in a 20x3 m
> space, and roughly the same number of client devices. The uplink is 3G
> based and has some inherent latency. Add some Bluetooth for flavour, stir
> gently. This one is rather challenging, but there is scope to optimise AP
> antenna placement, and to scale the test down slightly by reducing seat
> occupancy.
>
> 4) the jumbo jet, consisting of several hundred passengers crammed in like
> sardines. The uplink has satellite latencies built in. Good luck.
>
> 5) the business hotel. Multiple APs will be needed to provide adequate
> coverage for this environment, which should encompass the rooms as well as
> lounge, conference and dining areas. Some visitors may bring their own APs,
> and the system must be able to cope with this without seriously degrading
> performance.
>
> 6) the trade conference. A large arena filled with thousands of people.
> Multiple APs required. Good luck.
>
> I also feel that ultimately we're going to have to get industry on board.
> Not just passively letting us play around as with ath9k, but actively
> taking note of our findings and implementing at least a few of our ideas
> themselves. Of course, tools, models and real-world results are likely to
> make that easier.
>
> - Jonathan Morton
>
[-- Attachment #1.2: Type: text/html, Size: 12890 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Fu, Zhonghui @ 2015-02-02 1:58 UTC (permalink / raw)
To: Arend van Spriel, Kalle Valo
Cc: Sergei Shtylyov, brudley, Franky Lin, meuleman, linville,
pieterpg, hdegoede, wens, linux-wireless, brcm80211-dev-list,
netdev, linux-kernel@vger.kernel.org
In-Reply-To: <54C723A3.8000508@linux.intel.com>
On 2015/1/27 13:35, Fu, Zhonghui wrote:
> On 2015/1/24 2:09, Arend van Spriel wrote:
>> On 01/23/15 16:29, Kalle Valo wrote:
>>> Arend van Spriel<arend@broadcom.com> writes:
>>>
>>>> On 01/22/15 14:54, Sergei Shtylyov wrote:
>>>>> Hello.
>>>>>
>>>>> On 1/22/2015 4:49 PM, Kalle Valo wrote:
>>>>>
>>>>>>>> From 04d3fa673897ca4ccbea6c76836d0092dba2484a Mon Sep 17 00:00:00 2001
>>>>>>> From: Zhonghui Fu<zhonghui.fu@linux.intel.com>
>>>>>>> Date: Tue, 20 Jan 2015 11:14:13 +0800
>>>>>>> Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
>>>>>>> WiFi chip has 2 SDIO functions, and PM core will trigger
>>>>>>> twice suspend/resume operations for one WiFi chip to do
>>>>>>> the same things. This patch avoid this case.
>>>>>>> Acked-by: Arend van Spriel<arend@broadcom.com>
>>>>>>> Acked-by: Sergei Shtylyov<sergei.shtylyov@cogentembedded.com>
>>>>>>> Acked-by: Kalle Valo<kvalo@codeaurora.org>
>>>>>>> Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com>
>>>>>> I don't remember giving Acked-by to this (or for matter to anything for
>>>>>> a long time). What about Sergei or Arend?
>>>>> I haven't ACK'ed this patch either.
>>>> I did ACK the initial patch and felt it still valid for this 'V2' patch.
>>> Ok, thanks. So the patch is good, Zhonghui just needs to remove the two
>>> acked-by lines.
>> Indeed.
> I have removed the two acked-by lines and sent the new patch in another mail.
>
> Thanks,
> Zhonghui
What comments about the new patch? Can this new patch be accepted?
Thanks,
Zhonghui
>> Regards,
>> Arend
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Fixed TPACKET V3 to signal poll when block is closed rather than every packet
From: Guy Harris @ 2015-02-02 2:11 UTC (permalink / raw)
To: David Miller; +Cc: dan, netdev, linux-kernel
In-Reply-To: <20141222.154144.2133538994062884017.davem@davemloft.net>
On Dec 22, 2014, at 12:41 PM, David Miller <davem@davemloft.net> wrote:
> From: Dan Collins <dan@dcollins.co.nz>
> Date: Fri, 19 Dec 2014 16:49:25 +1300
>
>> Make TPACKET_V3 signal poll when block is closed rather than for every
>> packet. Side effect is that poll will be signaled when block retire
>> timer expires which didn't previously happen. Issue was visible when
>> sending packets at a very low frequency such that all blocks are retired
>> before packets are received by TPACKET_V3. This caused avoidable packet
>> loss. The fix ensures that the signal is sent when blocks are closed
>> which covers the normal path where the block is filled as well as the
>> path where the timer expires. The case where a block is filled without
>> moving to the next block (ie. all blocks are full) will still cause poll
>> to be signaled.
>>
>> Signed-off-by: Dan Collins <dan@dcollins.co.nz>
>
> Applied, thanks.
Should this be applied to any of the stable kernel branches? The old behavior is the source of some libpcap/tcpdump/etc. complaints; see, for example, the thread that included
http://marc.info/?l=linux-netdev&m=140713966510035&w=2
^ permalink raw reply
* [PATCHv2 net] net: restore lro after device detached from bridge
From: Fan Du @ 2015-02-02 2:20 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Fan Du, bhutchings, davem, netdev
In-Reply-To: <54CBEE24.8000603@redhat.com>
于 2015年01月31日 04:48, Alexander Duyck 写道:
> On 01/30/2015 04:33 AM, Fan Du wrote:
>> Either detaching a device from bridge or switching a device
>> out of FORWARDING state, the original lro feature should
>> possibly be enabled for good reason, e.g. hw feature like
>> receive side coalescing could come into play.
>>
>> BEFORE:
>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>> large-receive-offload: off
>>
>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>> large-receive-offload: off
>>
>> AFTER:
>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>> large-receive-offload: off
>>
>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k ens806f0 | grep large
>> large-receive-offload: on
>>
>> Signed-off-by: Fan Du <fan.du@intel.com>
>> Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are forwarding")
>
> First off this isn't a "fix". This is going to likely break more than
> it fixes. The main reason why LRO is disabled is because it can cause
> more harm then it helps. Since GRO is available we should err on the
> side of caution since enabling LRO/RSC can have undesirable side effects
> in a number of cases.
I think you are talking about bad scenarios when net device is attached to a bridge.
Then what's the good reason user has to pay extra cpu power for using GRO, instead
of using hw capable LRO/RSC when this net device is detached from bridge acting as
a standalone NIC?
Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as same other RSC capable
NICs. Attaching net device to a bridge _once_ should not changed its default configuration,
moreover it's a subtle change without any message that user won't noticed at all.
From 1e76b2625b3e6aa239b5ef8399fe441a587c6646 Mon Sep 17 00:00:00 2001
From: Fan Du <fan.du@intel.com>
Date: Mon, 2 Feb 2015 05:02:11 -0500
Subject: [PATCH] net: restore lro after device detached from bridge
When detached net device from a bridge, the original lro
feature should possibly be enabled for good reason, e.g.
hw feature like receive side coalescing could come into play.
Signed-off-by: Fan Du <fan.du@intel.com>
Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are forwarding")
---
ChangeLog:
v2:
- Restore lro only when device detached from bridge
---
include/linux/netdevice.h | 1 +
net/bridge/br_if.c | 1 +
net/core/dev.c | 23 +++++++++++++++++++++++
3 files changed, 25 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 642d426..904b1a4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2153,6 +2153,7 @@ int dev_alloc_name(struct net_device *dev, const char *name);
int dev_open(struct net_device *dev);
int dev_close(struct net_device *dev);
void dev_disable_lro(struct net_device *dev);
+void dev_enable_lro(struct net_device *dev);
int dev_loopback_xmit(struct sk_buff *newskb);
int dev_queue_xmit(struct sk_buff *skb);
int dev_queue_xmit_accel(struct sk_buff *skb, void *accel_priv);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 81e49fb..4236f3a 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -565,6 +565,7 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
netdev_update_features(br->dev);
+ dev_enable_lro(dev);
return 0;
}
diff --git a/net/core/dev.c b/net/core/dev.c
index 1e325ad..76f2ed7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1451,6 +1451,29 @@ void dev_disable_lro(struct net_device *dev)
}
EXPORT_SYMBOL(dev_disable_lro);
+/**
+ * dev_enable_lro - enable Large Receive Offload on a device
+ * @dev: device
+ *
+ * Enable Large Receive Offload (LRO) on a net device.
+ * This is needed if device is not attached to a bridge.
+ */
+void dev_enable_lro(struct net_device *dev)
+{
+ struct net_device *lower_dev;
+ struct list_head *iter;
+
+ dev->wanted_features |= NETIF_F_LRO;
+ netdev_update_features(dev);
+
+ if (unlikely(!(dev->features & NETIF_F_LRO)))
+ netdev_WARN(dev, "failed to enable LRO!\n");
+
+ netdev_for_each_lower_dev(dev, lower_dev, iter)
+ dev_enable_lro(lower_dev);
+}
+EXPORT_SYMBOL(dev_enable_lro);
+
static int call_netdevice_notifier(struct notifier_block *nb, unsigned long val,
struct net_device *dev)
{
--
1.8.3.1
> As far as the rest of the patch I have serious misgivings as this is
> going to be switching on LRO in multiple cases so if the user disables
> it they will find it was re-enabled when they likely weren't expecting
> it. LRO/RSC has a history of causing issues in a number of different
> cases. I'd say if it is off leave it off. If the user really wants to
> enable it they can do so via the ethtool interface that is already
> provided in the kernel after they have removed the interface from the
> bridge, or disabled IP routing.
>
> Leaving it disabled would be consistent with how it is handled in
> ixgbe_fix_features when Rx checksum offload is disabled. We just
> disable the feature and expect the user to re-enable it when they
> re-enable Rx checksum offload. The same logic should apply here. The
> user put the hardware in this state, it is the responsibility of the
> user to sort out the side effects of disabled features after they revert
> to their original state.
>> ---
>> include/linux/netdevice.h | 1 +
>> net/bridge/br_if.c | 1 +
>> net/core/dev.c | 24 ++++++++++++++++++++++++
>> net/ipv4/devinet.c | 4 ++++
>> net/ipv6/addrconf.c | 2 ++
>> 5 files changed, 32 insertions(+)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 642d426..904b1a4 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -2153,6 +2153,7 @@ int dev_alloc_name(struct net_device *dev, const char *name);
>> int dev_open(struct net_device *dev);
>> int dev_close(struct net_device *dev);
>> void dev_disable_lro(struct net_device *dev);
>> +void dev_enable_lro(struct net_device *dev);
>> int dev_loopback_xmit(struct sk_buff *newskb);
>> int dev_queue_xmit(struct sk_buff *skb);
>> int dev_queue_xmit_accel(struct sk_buff *skb, void *accel_priv);
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 81e49fb..4236f3a 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -565,6 +565,7 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
>> call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
>> netdev_update_features(br->dev);
>> + dev_enable_lro(dev);
>> return 0;
>> }
>
> Removing an interface from a bridge should not be grounds for turning on LRO/RSC.
>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 1e325ad..938d7f6 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1451,6 +1451,30 @@ void dev_disable_lro(struct net_device *dev)
>> }
>> EXPORT_SYMBOL(dev_disable_lro);
>> +/**
>> + * dev_enable_lro - enable Large Receive Offload on a device
>> + * @dev: device
>> + *
>> + * Enable Large Receive Offload (LRO) on a net device. Must be
>> + * called under RTNL. This is needed if device is not attached
>> + * to a bridge, or user change the forwarding state.
>> + */
>> +void dev_enable_lro(struct net_device *dev)
>> +{
>> + struct net_device *lower_dev;
>> + struct list_head *iter;
>> +
>> + dev->wanted_features |= NETIF_F_LRO;
>> + netdev_update_features(dev);
>> +
>> + if (unlikely(!(dev->features & NETIF_F_LRO)))
>> + netdev_WARN(dev, "failed to enable LRO!\n");
>> +
>> + netdev_for_each_lower_dev(dev, lower_dev, iter)
>> + dev_enable_lro(lower_dev);
>> +}
>> +EXPORT_SYMBOL(dev_enable_lro);
>> +
>> static int call_netdevice_notifier(struct notifier_block *nb, unsigned long val,
>> struct net_device *dev)
>> {
>> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
>> index 214882e..3307196 100644
>> --- a/net/ipv4/devinet.c
>> +++ b/net/ipv4/devinet.c
>> @@ -1956,6 +1956,8 @@ static void inet_forward_change(struct net *net)
>> struct in_device *in_dev;
>> if (on)
>> dev_disable_lro(dev);
>> + else
>> + dev_enable_lro(dev);
>> rcu_read_lock();
>> in_dev = __in_dev_get_rcu(dev);
>> if (in_dev) {
>
> Disabling it due to a feature conflict makes sense. Enabling it after disabling forwarding not so much. If the user disabled it prior to enabling forwarding it should stay disabled, not be enabled.
>
>> @@ -2047,6 +2049,8 @@ static int devinet_sysctl_forward(struct ctl_table *ctl, int write,
>> container_of(cnf, struct in_device, cnf);
>> if (*valp)
>> dev_disable_lro(idev->dev);
>> + else
>> + dev_enable_lro(idev->dev);
>> inet_netconf_notify_devconf(net,
>> NETCONFA_FORWARDING,
>> idev->dev->ifindex,
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index f7c8bbe..4c3b54c 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -669,6 +669,8 @@ static void dev_forward_change(struct inet6_dev *idev)
>> dev = idev->dev;
>> if (idev->cnf.forwarding)
>> dev_disable_lro(dev);
>> + else
>> + dev_enable_lro(dev);
>> if (dev->flags & IFF_MULTICAST) {
>> if (idev->cnf.forwarding) {
>> ipv6_dev_mc_inc(dev, &in6addr_linklocal_allrouters);
>
> Same applies here.
>
> If anything this patch seems like more of a feature request then a fix. It would likely create a desirable effect for some, but have some undesirable side-effects for other users.
>
> - Alex
^ permalink raw reply related
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2015-02-02 2:33 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Nicolas Dichtel, Thomas Graf
[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
drivers/net/vxlan.c between commit 33564bbb2cf1 ("vxlan: setup the
right link netns in newlink hdlr") from the net tree and commit
ac5132d1a03f ("vxlan: Only bind to sockets with compatible flags
enabled") from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/vxlan.c
index a8c755dcab14,31bac2a21ce3..000000000000
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@@ -2557,8 -2761,19 +2761,19 @@@ static int vxlan_newlink(struct net *sr
nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
vxlan->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
+ if (data[IFLA_VXLAN_REMCSUM_TX] &&
+ nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX]))
+ vxlan->flags |= VXLAN_F_REMCSUM_TX;
+
+ if (data[IFLA_VXLAN_REMCSUM_RX] &&
+ nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX]))
+ vxlan->flags |= VXLAN_F_REMCSUM_RX;
+
+ if (data[IFLA_VXLAN_GBP])
+ vxlan->flags |= VXLAN_F_GBP;
+
- if (vxlan_find_vni(net, vni, use_ipv6 ? AF_INET6 : AF_INET,
+ if (vxlan_find_vni(src_net, vni, use_ipv6 ? AF_INET6 : AF_INET,
- vxlan->dst_port)) {
+ vxlan->dst_port, vxlan->flags)) {
pr_info("duplicate VNI %u\n", vni);
return -EEXIST;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH net-next 1/7] r8152: adjust rx_bottom
From: Hayes Wang @ 2015-02-02 2:38 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2EE77A2@RTITMBSV03.realtek.com.tw>
> David Miller [mailto:davem@davemloft.net]
> > Sent: Sunday, January 25, 2015 2:44 PM
> > What keeps rtl_start_rx() from running in parallel with
> > r8152_submit_rx(), or any other accessor of the RX agg->list?
>
> Forgive my poor English. I would try to describe them clearly.
> The steps about the rx agg->list would be
> 1. carrier on or autoresume occurs.
> 2. Call rtl_start_rx().
> 3. Rx agg->list flows between device and tp->rx_done.
> 4. carrier off or autosuspend occurs.
> 5. call rtl_stop_rx().
>
> The rtl_start_rx() would only be called when the linking
> status is changed from off to on or the auto resume occurs.
> And rtl_start_rx() would reinitialize the tp->rx_done and
> all of the rx agg->list. After step 2, the rx agg->list
> would flow between the usb host controller and the driver.
> If r8152_submit_rx() is success, the driver wouldn't own the
> rx agg->list until it is returned from the usb host controller.
> If r8152_submit_rx() is fail, the driver would still own the
> rx agg->list, and queue it to the tp->rx_done with spin lock
> for next try.
>
> If the status stays in step 3, only the rx_bottom() would submit
> the rx agg. The rtl_start_rx() wouldn't be called suddenly,
> unless the linking down or auto suspend occur first and linking
> on or auto resume occur again. If linking down or auto suspend
> occur, rtl_stop_rx() would be called (step 5). After this step,
> rx_bottom() wouldn't submit rx, and all rx agg->list would stop
> flowing. That is, the tp->rx_done and all rx agg->list wouldn't
> be changed until the next rtl_start_rx() is called.
>
> Therefore, the flow for each rx agg->list would be
> a. submittd by rtl_start_rx().
> b. goto step c if success, otherwise goto step d.
> c. completed by usb host controller.
> d. queued to tp->rx_done with spin lock.
> e. dequeue from tp->rx_done with spin lock by rx_botoom().
> f. goto step i if link down, otherwise goto step g.
> g. submitted by rx_botoom().
> h. goto step b.
> i. goto step a if link on.
>
> And the patch change the step g to g1.
> g1. submitted by rx_botoom() if (!ret), otherwise goto step d.
Excuse me. Any other question or suggestion for this patch?
Best Regards,
Hayes
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2015-02-02 2:40 UTC (permalink / raw)
To: David Miller, netdev, Toshiaki Makita, Jiri Pirko
Cc: linux-next, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 516 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
include/linux/if_vlan.h between commit d4bcef3fbe88 ("net: Fix
vlan_get_protocol for stacked vlan") from the net tree and commit
df8a39defad4 ("net: rename vlan_tx_* helpers since "tx" is misleading
there") from the net-next tree.
I fixed it up (the former removed the code modified by the latter) and
can carry the fix as necessary (no action is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ 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