Re: [PATCH rdma-next v1] IB/rxe: Simplify rxe_find_route() to avoid GID query for netdev

[Yanjun Zhu <yanjun.zhu@xxxxxxxxxx>]

On 2018/8/28 18:51, Leon Romanovsky wrote:
> From: Parav Pandit <parav@xxxxxxxxxxxx>
>
> rxe_prepare() is called on an skb which has ndev already initialized by
> rxe_init_packet().
> Therefore avoid querying the GID attribute again and use the available
> netdevice from the skb->dev.
>
> Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
> Reviewed-by: Daniel Jurgens <danielj@xxxxxxxxxxxx>
> Reviewed-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
> Tested-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxxx>
Thanks a lot.
I am fine.

Zhu Yanjun
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> ---
> Changelog v0->v1:
>   * Added Yuval's tags
>   * Squashed Zhu's fixup patch
> ---
>   drivers/infiniband/sw/rxe/rxe_loc.h  |  3 +--
>   drivers/infiniband/sw/rxe/rxe_net.c  | 30 ++++++++++--------------------
>   drivers/infiniband/sw/rxe/rxe_req.c  |  2 +-
>   drivers/infiniband/sw/rxe/rxe_resp.c |  2 +-
>   4 files changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 87d14f7ef21b..8e305422adbb 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -144,8 +144,7 @@ void rxe_loopback(struct sk_buff *skb);
>   int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb);
>   struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
>   				int paylen, struct rxe_pkt_info *pkt);
> -int rxe_prepare(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
> -		struct sk_buff *skb, u32 *crc);
> +int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc);
>   enum rdma_link_layer rxe_link_layer(struct rxe_dev *rxe, unsigned int port_num);
>   const char *rxe_parent_name(struct rxe_dev *rxe, unsigned int port_num);
>   struct device *rxe_dma_device(struct rxe_dev *rxe);
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 8094cbaa54a9..08441a23022f 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -182,19 +182,11 @@ static struct dst_entry *rxe_find_route6(struct net_device *ndev,
>
>   #endif
>
> -static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
> +static struct dst_entry *rxe_find_route(struct net_device *ndev,
>   					struct rxe_qp *qp,
>   					struct rxe_av *av)
>   {
> -	const struct ib_gid_attr *attr;
>   	struct dst_entry *dst = NULL;
> -	struct net_device *ndev;
> -
> -	attr = rdma_get_gid_attr(&rxe->ib_dev, qp->attr.port_num,
> -				 av->grh.sgid_index);
> -	if (IS_ERR(attr))
> -		return NULL;
> -	ndev = attr->ndev;
>
>   	if (qp_type(qp) == IB_QPT_RC)
>   		dst = sk_dst_get(qp->sk->sk);
> @@ -229,7 +221,6 @@ static struct dst_entry *rxe_find_route(struct rxe_dev *rxe,
>   			sk_dst_set(qp->sk->sk, dst);
>   		}
>   	}
> -	rdma_put_gid_attr(attr);
>   	return dst;
>   }
>
> @@ -377,8 +368,8 @@ static void prepare_ipv6_hdr(struct dst_entry *dst, struct sk_buff *skb,
>   	ip6h->payload_len = htons(skb->len - sizeof(*ip6h));
>   }
>
> -static int prepare4(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
> -		    struct sk_buff *skb, struct rxe_av *av)
> +static int prepare4(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> +		    struct rxe_av *av)
>   {
>   	struct rxe_qp *qp = pkt->qp;
>   	struct dst_entry *dst;
> @@ -387,7 +378,7 @@ static int prepare4(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
>   	struct in_addr *saddr = &av->sgid_addr._sockaddr_in.sin_addr;
>   	struct in_addr *daddr = &av->dgid_addr._sockaddr_in.sin_addr;
>
> -	dst = rxe_find_route(rxe, qp, av);
> +	dst = rxe_find_route(skb->dev, qp, av);
>   	if (!dst) {
>   		pr_err("Host not reachable\n");
>   		return -EHOSTUNREACH;
> @@ -406,15 +397,15 @@ static int prepare4(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
>   	return 0;
>   }
>
> -static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
> -		    struct sk_buff *skb, struct rxe_av *av)
> +static int prepare6(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> +		    struct rxe_av *av)
>   {
>   	struct rxe_qp *qp = pkt->qp;
>   	struct dst_entry *dst;
>   	struct in6_addr *saddr = &av->sgid_addr._sockaddr_in6.sin6_addr;
>   	struct in6_addr *daddr = &av->dgid_addr._sockaddr_in6.sin6_addr;
>
> -	dst = rxe_find_route(rxe, qp, av);
> +	dst = rxe_find_route(skb->dev, qp, av);
>   	if (!dst) {
>   		pr_err("Host not reachable\n");
>   		return -EHOSTUNREACH;
> @@ -434,16 +425,15 @@ static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
>   	return 0;
>   }
>
> -int rxe_prepare(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
> -		struct sk_buff *skb, u32 *crc)
> +int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc)
>   {
>   	int err = 0;
>   	struct rxe_av *av = rxe_get_av(pkt);
>
>   	if (av->network_type == RDMA_NETWORK_IPV4)
> -		err = prepare4(rxe, pkt, skb, av);
> +		err = prepare4(pkt, skb, av);
>   	else if (av->network_type == RDMA_NETWORK_IPV6)
> -		err = prepare6(rxe, pkt, skb, av);
> +		err = prepare6(pkt, skb, av);
>
>   	*crc = rxe_icrc_hdr(pkt, skb);
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 8be27238a86e..84d35520781c 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -476,7 +476,7 @@ static int fill_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
>   	u32 *p;
>   	int err;
>
> -	err = rxe_prepare(rxe, pkt, skb, &crc);
> +	err = rxe_prepare(pkt, skb, &crc);
>   	if (err)
>   		return err;
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index aa5833318372..c1948d406528 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -637,7 +637,7 @@ static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
>   	if (ack->mask & RXE_ATMACK_MASK)
>   		atmack_set_orig(ack, qp->resp.atomic_orig);
>
> -	err = rxe_prepare(rxe, ack, skb, &crc);
> +	err = rxe_prepare(ack, skb, &crc);
>   	if (err) {
>   		kfree_skb(skb);
>   		return NULL;
> --
> 2.14.4