Message ID | 20120713165221.28140.92681.stgit@gitlad.jf.intel.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, 2012-07-13 at 09:55 -0700, Alexander Duyck wrote: > The ethtool function for getting the rule count was not zeroing out the > data field before passing it to the kernel. As a result the value started > uninitialized and was incorrectly returning a result indicating that > devices supported setting new rule indexes. In order to correct this I am > adding a one line fix that sets data to zero before we pass the command to > the kernel. Right. For 'get' commands with no parameters (besides the device) the data copied back to userland is normally zero-initialised and then filled out by the driver, and I seem to have worked on that assumption. But because of the odd multiplexing of RX NFC commands ETHTOOL_GRXCLSRLCNT doesn't work like that. And for 'my' driver that didn't matter. Sorry about that. (We should really have some explicit documentation of responsibility for structure initialisation.) > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com> > --- > > I am resending this since I didn't see any notification that it had been seen. > I also realized that I had not clearly identified that this is an ethtool user > space patch and not an ethtool kernel space patch. It was perfectly clear and I had queued it up to review but hadn't yet done so. Ben. > rxclass.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/rxclass.c b/rxclass.c > index 4d49aa6..e1633a8 100644 > --- a/rxclass.c > +++ b/rxclass.c > @@ -207,6 +207,7 @@ static int rxclass_get_dev_info(struct cmd_context *ctx, __u32 *count, > int err; > > nfccmd.cmd = ETHTOOL_GRXCLSRLCNT; > + nfccmd.data = 0; > err = send_ioctl(ctx, &nfccmd); > *count = nfccmd.rule_cnt; > if (driver_select) >
On 07/16/2012 01:03 PM, Ben Hutchings wrote: > On Fri, 2012-07-13 at 09:55 -0700, Alexander Duyck wrote: >> The ethtool function for getting the rule count was not zeroing out the >> data field before passing it to the kernel. As a result the value started >> uninitialized and was incorrectly returning a result indicating that >> devices supported setting new rule indexes. In order to correct this I am >> adding a one line fix that sets data to zero before we pass the command to >> the kernel. > Right. For 'get' commands with no parameters (besides the device) the > data copied back to userland is normally zero-initialised and then > filled out by the driver, and I seem to have worked on that assumption. > But because of the odd multiplexing of RX NFC commands > ETHTOOL_GRXCLSRLCNT doesn't work like that. And for 'my' driver that > didn't matter. Sorry about that. > > (We should really have some explicit documentation of responsibility for > structure initialisation.) > >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com> >> --- >> >> I am resending this since I didn't see any notification that it had been seen. >> I also realized that I had not clearly identified that this is an ethtool user >> space patch and not an ethtool kernel space patch. > It was perfectly clear and I had queued it up to review but hadn't yet > done so. > > Ben. > Yeah, that was my mistake. I thought I hadn't sent it out with the ethtool prefix when I actually had. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2012-07-16 at 17:10 -0700, Alexander Duyck wrote: > On 07/16/2012 01:03 PM, Ben Hutchings wrote: > > On Fri, 2012-07-13 at 09:55 -0700, Alexander Duyck wrote: > >> The ethtool function for getting the rule count was not zeroing out the > >> data field before passing it to the kernel. As a result the value started > >> uninitialized and was incorrectly returning a result indicating that > >> devices supported setting new rule indexes. In order to correct this I am > >> adding a one line fix that sets data to zero before we pass the command to > >> the kernel. > > Right. For 'get' commands with no parameters (besides the device) the > > data copied back to userland is normally zero-initialised and then > > filled out by the driver, and I seem to have worked on that assumption. > > But because of the odd multiplexing of RX NFC commands > > ETHTOOL_GRXCLSRLCNT doesn't work like that. And for 'my' driver that > > didn't matter. Sorry about that. > > > > (We should really have some explicit documentation of responsibility for > > structure initialisation.) > > > >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com> > >> --- > >> > >> I am resending this since I didn't see any notification that it had been seen. > >> I also realized that I had not clearly identified that this is an ethtool user > >> space patch and not an ethtool kernel space patch. > > It was perfectly clear and I had queued it up to review but hadn't yet > > done so. > > > > Ben. > > > Yeah, that was my mistake. I thought I hadn't sent it out with the > ethtool prefix when I actually had. So, anyway, I've applied it and just done a bug fix release (3.4.2). Ben.
diff --git a/rxclass.c b/rxclass.c index 4d49aa6..e1633a8 100644 --- a/rxclass.c +++ b/rxclass.c @@ -207,6 +207,7 @@ static int rxclass_get_dev_info(struct cmd_context *ctx, __u32 *count, int err; nfccmd.cmd = ETHTOOL_GRXCLSRLCNT; + nfccmd.data = 0; err = send_ioctl(ctx, &nfccmd); *count = nfccmd.rule_cnt; if (driver_select)
The ethtool function for getting the rule count was not zeroing out the data field before passing it to the kernel. As a result the value started uninitialized and was incorrectly returning a result indicating that devices supported setting new rule indexes. In order to correct this I am adding a one line fix that sets data to zero before we pass the command to the kernel. Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com> --- I am resending this since I didn't see any notification that it had been seen. I also realized that I had not clearly identified that this is an ethtool user space patch and not an ethtool kernel space patch. rxclass.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html