All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] IB/mlx5: add checking for "vf" from do_setvfinfo()
Date: Mon, 22 Apr 2019 08:08:36 +0000	[thread overview]
Message-ID: <20190420095102.GA14798@kadam> (raw)
In-Reply-To: <20190412175504.GA20857@kadam>

On Tue, Apr 16, 2019 at 10:54:42PM +0000, Parav Pandit wrote:
> > Yeah.  But the call tree here is:
> > 
> > do_setvfinfo()
> > -> ops->ndo_get_vf_config()
> >    -> rtnl_fill_vfinfo()
> >       -> dev->netdev_ops->ndo_get_vf_config(dev, vfs_num, &ivi)
> >          -> ipoib_get_vf_config()
> >             -> ib_get_vf_config
> >                -> device->ops.get_vf_config(device, vf, port, info);
> > 
> > Changing the ->ndo_get_vf_config() pointer means you have to update 20
> > functions in various drivers.  It becomes quite involved.  We should apply
> > this simple self contained fix then worry about doing other cleanups later.
> > 
> But if a static checker is run on following functions, they need for vf < 0 check.
> 
> i40e_ndo_get_vf_config
> mlx5e_get_vf_config
> bnxt_get_vf_config
> etc and few more.

I checked again to see if it was "20 functions" or if it was "etc and
few more"...  It turns out its only 18 functions because I double
counted two functions at first.  Here is the list:

be_get_vf_config
bnx2x_get_vf_config
bnxt_get_vf_config
cxgb4_mgmt_get_vf_config
efx_sriov_get_vf_config
fm10k_ndo_get_vf_config
i40e_ndo_get_vf_config
ice_get_vf_cfg
igb_ndo_get_vf_config
ipoib_get_vf_config
ixgbe_ndo_get_vf_config
liquidio_get_vf_config
mlx4_en_get_vf_config
mlx5e_get_vf_config
nfp_app_get_vf_config
nsim_get_vf_config
qede_get_vf_config
qlcnic_sriov_get_vf_config

But you also have to update the call tress as well...  It's really very
involved.  I seriously did look at how to do this and the original patch
is the Right Thing To Do [tm].  I've probably sent around 92 underflow
patches and sometimes I would definitely agree with you that changing
the type is the right fix but not in this case.

regards,
dan carpenter

  parent reply	other threads:[~2019-04-22  8:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-12 17:55 [PATCH] IB/mlx5: add checking for "vf" from do_setvfinfo() Dan Carpenter
2019-04-12 20:25 ` Parav Pandit
2019-04-15  9:32 ` Dan Carpenter
2019-04-15  9:46 ` Dan Carpenter
2019-04-15 20:04 ` Parav Pandit
2019-04-16  8:21 ` Dan Carpenter
2019-04-16 22:54 ` Parav Pandit
2019-04-22  8:08 ` Dan Carpenter [this message]
2019-04-22 15:09 ` Parav Pandit
2019-04-23 15:49 ` Dan Carpenter
2019-04-23 22:32 ` Parav Pandit
2019-04-24 14:08   ` Dan Carpenter
2019-04-24 14:08     ` Dan Carpenter
2019-04-24 14:24     ` Jason Gunthorpe
2019-04-24 14:24       ` Jason Gunthorpe
2019-04-24 22:12       ` Parav Pandit
2019-04-25  0:36     ` Jakub Kicinski
2019-04-25  0:36       ` Jakub Kicinski
2019-04-25  6:15     ` Parav Pandit
2019-09-24  9:21       ` Dan Carpenter
2019-09-24  9:21         ` Dan Carpenter
2019-09-25 17:14         ` Parav Pandit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190420095102.GA14798@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.