| Message ID | 20190530131817.6147-1-kamalheib1@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | cbdc666f3e842b01d4537933e0a64f1e7cf17017 |
| Delegated to: | Doug Ledford |
| Headers | show |
| Series | [for-next] RDMA/ipoib: Remove check for ETH_SS_TEST | expand |
On Thu, May 30, 2019 at 04:18:17PM +0300, Kamal Heib wrote: > Self-test isn't supported by the ipoib driver, so remove the check for > ETH_SS_TEST. > > Fixes: e3614bc9dc44 ("IB/ipoib: Add readout of statistics using ethtool") > Signed-off-by: Kamal Heib <kamalheib1@gmail.com> > --- > drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > index 83429925dfc6..b0bd0ff0b45c 100644 > --- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > @@ -138,7 +138,6 @@ static void ipoib_get_strings(struct net_device __always_unused *dev, > p += ETH_GSTRING_LEN; > } > break; > - case ETH_SS_TEST: The commit message and code doesn't match each other. Removing this specific case will leave exactly the same behaviour as before, so why should we change it? > default: > break; > } > @@ -149,7 +148,6 @@ static int ipoib_get_sset_count(struct net_device __always_unused *dev, > switch (sset) { > case ETH_SS_STATS: > return IPOIB_GLOBAL_STATS_LEN; > - case ETH_SS_TEST: > default: > break; > } > -- > 2.20.1 >
On Fri, 2019-06-07 at 15:09 +0300, Leon Romanovsky wrote: > On Thu, May 30, 2019 at 04:18:17PM +0300, Kamal Heib wrote: > > Self-test isn't supported by the ipoib driver, so remove the check > > for > > ETH_SS_TEST. > > > > Fixes: e3614bc9dc44 ("IB/ipoib: Add readout of statistics using > > ethtool") > > Signed-off-by: Kamal Heib <kamalheib1@gmail.com> > > --- > > drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > index 83429925dfc6..b0bd0ff0b45c 100644 > > --- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > @@ -138,7 +138,6 @@ static void ipoib_get_strings(struct net_device > > __always_unused *dev, > > p += ETH_GSTRING_LEN; > > } > > break; > > - case ETH_SS_TEST: > > The commit message and code doesn't match each other. > Removing this specific case will leave exactly the same behaviour as > before, so why should we change it? > The idea is very simple, no point of checking ETH_SS_TEST if the ipoib doesn't support it. > > default: > > break; > > } > > @@ -149,7 +148,6 @@ static int ipoib_get_sset_count(struct > > net_device __always_unused *dev, > > switch (sset) { > > case ETH_SS_STATS: > > return IPOIB_GLOBAL_STATS_LEN; > > - case ETH_SS_TEST: > > default: > > break; > > } > > -- > > 2.20.1 > >
On Mon, Jun 10, 2019 at 01:59:31PM +0300, Kamal Heib wrote: > On Fri, 2019-06-07 at 15:09 +0300, Leon Romanovsky wrote: > > On Thu, May 30, 2019 at 04:18:17PM +0300, Kamal Heib wrote: > > > Self-test isn't supported by the ipoib driver, so remove the check > > > for > > > ETH_SS_TEST. > > > > > > Fixes: e3614bc9dc44 ("IB/ipoib: Add readout of statistics using > > > ethtool") > > > Signed-off-by: Kamal Heib <kamalheib1@gmail.com> > > > --- > > > drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > index 83429925dfc6..b0bd0ff0b45c 100644 > > > --- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > @@ -138,7 +138,6 @@ static void ipoib_get_strings(struct net_device > > > __always_unused *dev, > > > p += ETH_GSTRING_LEN; > > > } > > > break; > > > - case ETH_SS_TEST: > > > > The commit message and code doesn't match each other. > > Removing this specific case will leave exactly the same behaviour as > > before, so why should we change it? > > > > The idea is very simple, no point of checking ETH_SS_TEST if the ipoib > doesn't support it. Please write in commit message, that "default" option means "unsupported" and there is no need in explicit declaration of unsupported ETH_SS_TEST. Thanks
On Mon, 2019-06-10 at 16:12 +0300, Leon Romanovsky wrote: > On Mon, Jun 10, 2019 at 01:59:31PM +0300, Kamal Heib wrote: > > On Fri, 2019-06-07 at 15:09 +0300, Leon Romanovsky wrote: > > > On Thu, May 30, 2019 at 04:18:17PM +0300, Kamal Heib wrote: > > > > Self-test isn't supported by the ipoib driver, so remove the > > > > check > > > > for > > > > ETH_SS_TEST. > > > > > > > > Fixes: e3614bc9dc44 ("IB/ipoib: Add readout of statistics using > > > > ethtool") > > > > Signed-off-by: Kamal Heib <kamalheib1@gmail.com> > > > > --- > > > > drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 2 -- > > > > 1 file changed, 2 deletions(-) > > > > > > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > > b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > > index 83429925dfc6..b0bd0ff0b45c 100644 > > > > --- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c > > > > @@ -138,7 +138,6 @@ static void ipoib_get_strings(struct > > > > net_device > > > > __always_unused *dev, > > > > p += ETH_GSTRING_LEN; > > > > } > > > > break; > > > > - case ETH_SS_TEST: > > > > > > The commit message and code doesn't match each other. > > > Removing this specific case will leave exactly the same behaviour > > > as > > > before, so why should we change it? > > > > > > > The idea is very simple, no point of checking ETH_SS_TEST if the > > ipoib > > doesn't support it. > > Please write in commit message, that "default" option means > "unsupported" and > there is no need in explicit declaration of unsupported ETH_SS_TEST. > > Thanks With an appropriate fix to the commit message, applied to for-next, thanks.
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c index 83429925dfc6..b0bd0ff0b45c 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c @@ -138,7 +138,6 @@ static void ipoib_get_strings(struct net_device __always_unused *dev, p += ETH_GSTRING_LEN; } break; - case ETH_SS_TEST: default: break; } @@ -149,7 +148,6 @@ static int ipoib_get_sset_count(struct net_device __always_unused *dev, switch (sset) { case ETH_SS_STATS: return IPOIB_GLOBAL_STATS_LEN; - case ETH_SS_TEST: default: break; }
Self-test isn't supported by the ipoib driver, so remove the check for ETH_SS_TEST. Fixes: e3614bc9dc44 ("IB/ipoib: Add readout of statistics using ethtool") Signed-off-by: Kamal Heib <kamalheib1@gmail.com> --- drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 2 -- 1 file changed, 2 deletions(-)