Re: [RFC PATCH 2/3] net/ncsi: Fix several packet definitions

[Samuel Mendoza-Jonas <sam@mendozajonas.com>]

On Thu, 2017-08-10 at 16:33 +0930, Joel Stanley wrote:
> On Wed, Aug 9, 2017 at 6:24 PM, Samuel Mendoza-Jonas
> <sam@mendozajonas.com> wrote:
> > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > ---
> >  net/ncsi/ncsi-cmd.c | 10 +++++-----
> >  net/ncsi/ncsi-pkt.h |  2 +-
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> > index db7083bfd476..1fec9fda7f60 100644
> > --- a/net/ncsi/ncsi-cmd.c
> > +++ b/net/ncsi/ncsi-cmd.c
> > @@ -146,9 +146,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb,
> > 
> >         cmd = (struct ncsi_cmd_svf_pkt *)skb_put(skb, sizeof(*cmd));
> >         memset(cmd, 0, sizeof(*cmd));
> > -       cmd->vlan = htons(nca->words[0]);
> > -       cmd->index = nca->bytes[2];
> > -       cmd->enable = nca->bytes[3];
> > +       cmd->vlan = htons(nca->words[1]);
> > +       cmd->index = nca->bytes[6];
> > +       cmd->enable = nca->bytes[7];
> 
> These look like straight up bugs. Should we send them off as fixes?

These are straight up bugs except... without my changes we never call
this code. As Ben says as time provides a lot of the current definitions
need to be gone over, there's a few command/response code paths that are
never triggered and could be broken in similar ways.

> 
> >         ncsi_cmd_build_header(&cmd->cmd.common, nca);
> > 
> >         return 0;
> > @@ -161,7 +161,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb,
> > 
> >         cmd = (struct ncsi_cmd_ev_pkt *)skb_put(skb, sizeof(*cmd));
> >         memset(cmd, 0, sizeof(*cmd));
> > -       cmd->mode = nca->bytes[0];
> > +       cmd->mode = nca->bytes[3];
> >         ncsi_cmd_build_header(&cmd->cmd.common, nca);
> > 
> >         return 0;
> > @@ -240,7 +240,7 @@ static struct ncsi_cmd_handler {
> >         { NCSI_PKT_CMD_AE,     8, ncsi_cmd_handler_ae      },
> >         { NCSI_PKT_CMD_SL,     8, ncsi_cmd_handler_sl      },
> >         { NCSI_PKT_CMD_GLS,    0, ncsi_cmd_handler_default },
> > -       { NCSI_PKT_CMD_SVF,    4, ncsi_cmd_handler_svf     },
> > +       { NCSI_PKT_CMD_SVF,    8, ncsi_cmd_handler_svf     },
> >         { NCSI_PKT_CMD_EV,     4, ncsi_cmd_handler_ev      },
> >         { NCSI_PKT_CMD_DV,     0, ncsi_cmd_handler_default },
> >         { NCSI_PKT_CMD_SMA,    8, ncsi_cmd_handler_sma     },
> > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> > index 3ea49ed0a935..91b4b66438df 100644
> > --- a/net/ncsi/ncsi-pkt.h
> > +++ b/net/ncsi/ncsi-pkt.h
> > @@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt {
> >         unsigned char           index;     /* VLAN table index  */
> >         unsigned char           enable;    /* Enable or disable */
> >         __be32                  checksum;  /* Checksum          */
> > -       unsigned char           pad[14];
> > +       unsigned char           pad[18];
> >  };
> > 
> >  /* Enable VLAN */
> > --
> > 2.13.3
> >