RE: [PATCH 1/4] RDMA/core: Squash a single user static function

Parav Pandit
2025-09-10T10:51:11Z

> From: Leon Romanovsky <leon@kernel.org>
> Sent: 10 September 2025 01:48 PM
>
> On Sun, Sep 07, 2025 at 07:08:30PM +0300, Edward Srouji wrote:
> > From: Parav Pandit <parav@nvidia.com>
> >
> > In order to reduce dependencies in IFF_LOOPBACK in route and neighbour
> > resolution steps, squash the static function to its single caller and
> > simplify the code. No functional change.
>
> It needs more explanation why it is true.
After second look at the code, it is not true.
It is not true for the case when dev->flags has IFF_LOOPBACK and translate_ip() failed.
In existing code, when translate_ip() fails, code still sets dev_addr->network.
dev_addr->network is not referred if the process_one_req() has error.

> Before this change, we set dev_addr-
> >network to some value and returned error.
> That error propagated upto process_one_req(), which handles only some
> errors and ignores rest.
>
> So now, we continue to handle REQ without proper req->addr->network.
>
Not exactly. When error is returned by the code addr->network is not filled up, which is correct thing to do.

So at best, commit message should be updated to remove "No functional change".

Will send v1.

> Thanks
>
> >
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > Reviewed-by: Vlad Dumitrescu <vdumitrescu@nvidia.com>
> > Signed-off-by: Edward Srouji <edwards@nvidia.com>
> > ---
> > drivers/infiniband/core/addr.c | 49
> > ++++++++++++++--------------------
> > 1 file changed, 20 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/addr.c
> > b/drivers/infiniband/core/addr.c index be0743dac3ff..594e7ee335f7
> > 100644
> > --- a/drivers/infiniband/core/addr.c
> > +++ b/drivers/infiniband/core/addr.c
> > @@ -465,34 +465,6 @@ static int addr_resolve_neigh(const struct dst_entry
> *dst,
> > return ret;
> > }
> >
> > -static int copy_src_l2_addr(struct rdma_dev_addr *dev_addr,
> > - const struct sockaddr *dst_in,
> > - const struct dst_entry *dst,
> > - const struct net_device *ndev)
> > -{
> > - int ret = 0;
> > -
> > - if (dst->dev->flags & IFF_LOOPBACK)
> > - ret = rdma_translate_ip(dst_in, dev_addr);
> > - else
> > - rdma_copy_src_l2_addr(dev_addr, dst->dev);
> > -
> > - /*
> > - * If there's a gateway and type of device not ARPHRD_INFINIBAND,
> > - * we're definitely in RoCE v2 (as RoCE v1 isn't routable) set the
> > - * network type accordingly.
> > - */
> > - if (has_gateway(dst, dst_in->sa_family) &&
> > - ndev->type != ARPHRD_INFINIBAND)
> > - dev_addr->network = dst_in->sa_family == AF_INET ?
> > - RDMA_NETWORK_IPV4 :
> > - RDMA_NETWORK_IPV6;
> > - else
> > - dev_addr->network = RDMA_NETWORK_IB;
> > -
> > - return ret;
> > -}
> > -
> > static int rdma_set_src_addr_rcu(struct rdma_dev_addr *dev_addr,
> > unsigned int *ndev_flags,
> > const struct sockaddr *dst_in,
> > @@ -503,6 +475,7 @@ static int rdma_set_src_addr_rcu(struct
> rdma_dev_addr *dev_addr,
> > *ndev_flags = ndev->flags;
> > /* A physical device must be the RDMA device to use */
> > if (ndev->flags & IFF_LOOPBACK) {
> > + int ret;
> > /*
> > * RDMA (IB/RoCE, iWarp) doesn't run on lo interface or
> > * loopback IP address. So if route is resolved to loopback
> @@
> > -512,9 +485,27 @@ static int rdma_set_src_addr_rcu(struct rdma_dev_addr
> *dev_addr,
> > ndev = rdma_find_ndev_for_src_ip_rcu(dev_net(ndev),
> dst_in);
> > if (IS_ERR(ndev))
> > return -ENODEV;
> > + ret = rdma_translate_ip(dst_in, dev_addr);
> > + if (ret)
> > + return ret;
> > + } else {
> > + rdma_copy_src_l2_addr(dev_addr, dst->dev);
> > }
> >
> > - return copy_src_l2_addr(dev_addr, dst_in, dst, ndev);
> > + /*
> > + * If there's a gateway and type of device not ARPHRD_INFINIBAND,
> > + * we're definitely in RoCE v2 (as RoCE v1 isn't routable) set the
> > + * network type accordingly.
> > + */
> > + if (has_gateway(dst, dst_in->sa_family) &&
> > + ndev->type != ARPHRD_INFINIBAND)
> > + dev_addr->network = dst_in->sa_family == AF_INET ?
> > + RDMA_NETWORK_IPV4 :
> > + RDMA_NETWORK_IPV6;
> > + else
> > + dev_addr->network = RDMA_NETWORK_IB;
> > +
> > + return 0;
> > }
> >
> > static int set_addr_netns_by_gid_rcu(struct rdma_dev_addr *addr)
> > --
> > 2.21.3
> >
> >