| Message ID | 20181206125212.4452-1-yuval.shaia@oracle.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | IB/rxe: Omit port validation from IB verbs | expand |
On Thu, Dec 06, 2018 at 02:52:12PM +0200, Yuval Shaia wrote: > RDMA core layer already make sure port is valid, no need to check it > here again. > > For the pkey validation this depends on commit b3ac5742fead ("RDMA/core: > Validate port number in query_pkey verb") > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> > --- > drivers/infiniband/sw/rxe/rxe_verbs.c | 21 +-------------------- > 1 file changed, 1 insertion(+), 20 deletions(-) Can you check if other drivers have the same redunent check and remove it too? Jason
On 2018/12/6 20:52, Yuval Shaia wrote: > RDMA core layer already make sure port is valid, no need to check it > here again. > > For the pkey validation this depends on commit b3ac5742fead ("RDMA/core: > Validate port number in query_pkey verb") > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> > --- > drivers/infiniband/sw/rxe/rxe_verbs.c | 21 +-------------------- > 1 file changed, 1 insertion(+), 20 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c > index 30817c79ba96..65c05b0126bd 100644 > --- a/drivers/infiniband/sw/rxe/rxe_verbs.c > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c > @@ -56,12 +56,7 @@ static int rxe_query_port(struct ib_device *dev, > { > struct rxe_dev *rxe = to_rdev(dev); > struct rxe_port *port; > - int rc = -EINVAL; > - > - if (unlikely(port_num != 1)) { > - pr_warn("invalid port_number %d\n", port_num); > - goto out; > - } > + int rc; > > port = &rxe->port; static int rxe_query_port(struct ib_device *dev, u8 port_num, struct ib_port_attr *attr) { struct rxe_dev *rxe = to_rdev(dev); struct rxe_port *port; int rc; port = &rxe->port; /* *attr being zeroed by the caller, avoid zeroing it here */ *attr = port->attr; mutex_lock(&rxe->usdev_lock); rc = ib_get_eth_speed(dev, port_num, &attr->active_speed, &attr->active_width); mutex_unlock(&rxe->usdev_lock); out: <----This tag should be removed since this tag is not used again. return rc; } Zhu Yanjun > > @@ -104,12 +99,6 @@ static int rxe_query_pkey(struct ib_device *device, > struct rxe_dev *rxe = to_rdev(device); > struct rxe_port *port; > > - if (unlikely(port_num != 1)) { > - dev_warn(device->dev.parent, "invalid port_num = %d\n", > - port_num); > - goto err1; > - } > - > port = &rxe->port; > > if (unlikely(index >= port->attr.pkey_tbl_len)) { > @@ -147,11 +136,6 @@ static int rxe_modify_port(struct ib_device *dev, > struct rxe_dev *rxe = to_rdev(dev); > struct rxe_port *port; > > - if (unlikely(port_num != 1)) { > - pr_warn("invalid port_num = %d\n", port_num); > - goto err1; > - } > - > port = &rxe->port; > > port->attr.port_cap_flags |= attr->set_port_cap_mask; > @@ -161,9 +145,6 @@ static int rxe_modify_port(struct ib_device *dev, > port->attr.qkey_viol_cntr = 0; > > return 0; > - > -err1: > - return -EINVAL; > } > > static enum rdma_link_layer rxe_get_link_layer(struct ib_device *dev,
On Fri, Dec 07, 2018 at 10:17:02AM +0800, Yanjun Zhu wrote: > > On 2018/12/6 20:52, Yuval Shaia wrote: > > RDMA core layer already make sure port is valid, no need to check it > > here again. > > > > For the pkey validation this depends on commit b3ac5742fead ("RDMA/core: > > Validate port number in query_pkey verb") > > > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> > > --- > > drivers/infiniband/sw/rxe/rxe_verbs.c | 21 +-------------------- > > 1 file changed, 1 insertion(+), 20 deletions(-) > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c > > index 30817c79ba96..65c05b0126bd 100644 > > --- a/drivers/infiniband/sw/rxe/rxe_verbs.c > > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c > > @@ -56,12 +56,7 @@ static int rxe_query_port(struct ib_device *dev, > > { > > struct rxe_dev *rxe = to_rdev(dev); > > struct rxe_port *port; > > - int rc = -EINVAL; > > - > > - if (unlikely(port_num != 1)) { > > - pr_warn("invalid port_number %d\n", port_num); > > - goto out; > > - } > > + int rc; > > port = &rxe->port; > static int rxe_query_port(struct ib_device *dev, > u8 port_num, struct ib_port_attr *attr) > { > struct rxe_dev *rxe = to_rdev(dev); > struct rxe_port *port; > int rc; > > port = &rxe->port; > > /* *attr being zeroed by the caller, avoid zeroing it here */ > *attr = port->attr; > > mutex_lock(&rxe->usdev_lock); > rc = ib_get_eth_speed(dev, port_num, &attr->active_speed, > &attr->active_width); > mutex_unlock(&rxe->usdev_lock); > > out: <----This tag should be removed since this tag is not used again. Thanks! (I wonder why the compiler didn't catch this) > > return rc; > } > > > Zhu Yanjun > > > @@ -104,12 +99,6 @@ static int rxe_query_pkey(struct ib_device *device, > > struct rxe_dev *rxe = to_rdev(device); > > struct rxe_port *port; > > - if (unlikely(port_num != 1)) { > > - dev_warn(device->dev.parent, "invalid port_num = %d\n", > > - port_num); > > - goto err1; > > - } > > - > > port = &rxe->port; > > if (unlikely(index >= port->attr.pkey_tbl_len)) { > > @@ -147,11 +136,6 @@ static int rxe_modify_port(struct ib_device *dev, > > struct rxe_dev *rxe = to_rdev(dev); > > struct rxe_port *port; > > - if (unlikely(port_num != 1)) { > > - pr_warn("invalid port_num = %d\n", port_num); > > - goto err1; > > - } > > - > > port = &rxe->port; > > port->attr.port_cap_flags |= attr->set_port_cap_mask; > > @@ -161,9 +145,6 @@ static int rxe_modify_port(struct ib_device *dev, > > port->attr.qkey_viol_cntr = 0; > > return 0; > > - > > -err1: > > - return -EINVAL; > > } > > static enum rdma_link_layer rxe_get_link_layer(struct ib_device *dev,
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c index 30817c79ba96..65c05b0126bd 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.c +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c @@ -56,12 +56,7 @@ static int rxe_query_port(struct ib_device *dev, { struct rxe_dev *rxe = to_rdev(dev); struct rxe_port *port; - int rc = -EINVAL; - - if (unlikely(port_num != 1)) { - pr_warn("invalid port_number %d\n", port_num); - goto out; - } + int rc; port = &rxe->port; @@ -104,12 +99,6 @@ static int rxe_query_pkey(struct ib_device *device, struct rxe_dev *rxe = to_rdev(device); struct rxe_port *port; - if (unlikely(port_num != 1)) { - dev_warn(device->dev.parent, "invalid port_num = %d\n", - port_num); - goto err1; - } - port = &rxe->port; if (unlikely(index >= port->attr.pkey_tbl_len)) { @@ -147,11 +136,6 @@ static int rxe_modify_port(struct ib_device *dev, struct rxe_dev *rxe = to_rdev(dev); struct rxe_port *port; - if (unlikely(port_num != 1)) { - pr_warn("invalid port_num = %d\n", port_num); - goto err1; - } - port = &rxe->port; port->attr.port_cap_flags |= attr->set_port_cap_mask; @@ -161,9 +145,6 @@ static int rxe_modify_port(struct ib_device *dev, port->attr.qkey_viol_cntr = 0; return 0; - -err1: - return -EINVAL; } static enum rdma_link_layer rxe_get_link_layer(struct ib_device *dev,
RDMA core layer already make sure port is valid, no need to check it here again. For the pkey validation this depends on commit b3ac5742fead ("RDMA/core: Validate port number in query_pkey verb") Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> --- drivers/infiniband/sw/rxe/rxe_verbs.c | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-)