RE: [PATCH] net/ncsi: add control packet payload to NC-SI commands from netlink

[Justin.Lee1]

Hi Ben,

I have similar fix locally with different approach as the command handler may 
have some expectation for those byes.
We can use NCSI_PKT_CMD_OEM handler as it only copies data based on the payload 
length.

diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 5c3fad8..3b01f65 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -309,14 +309,19 @@ static struct ncsi_request *ncsi_alloc_command(struct 
ncsi_cmd_arg *nca)
 
 int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
 {
+ struct ncsi_cmd_handler *nch = NULL;
        struct ncsi_request *nr;
+ unsigned char type;
        struct ethhdr *eh;
-   struct ncsi_cmd_handler *nch = NULL;
        int i, ret;
 
+ if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN)
+         type = NCSI_PKT_CMD_OEM;
+ else
+         type = nca->type;
        /* Search for the handler */
        for (i = 0; i < ARRAY_SIZE(ncsi_cmd_handlers); i++) {
-           if (ncsi_cmd_handlers[i].type == nca->type) {
+         if (ncsi_cmd_handlers[i].type == type) {
                        if (ncsi_cmd_handlers[i].handler)
                                nch = &ncsi_cmd_handlers[i];
                        else

Thanks,
Justin



> This patch adds support for NCSI_CMD_SEND_CMD netlink command to load packet 
> data payload (up to 16 bytes) to standard NC-SI commands.
> 
> Packet data will be loaded from NCSI_ATTR_DATA attribute similar to NC-SI OEM 
> commands
> 
> Signed-off-by: Ben Wei <ben...@fb.com>
> ---
>  net/ncsi/internal.h     | 7 ++++---
>  net/ncsi/ncsi-netlink.c | 9 +++++++++
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h index 
> 0b3f0673e1a2..4ff442faf5dc 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -328,9 +328,10 @@ struct ncsi_cmd_arg {
>       unsigned short       payload;     /* Command packet payload length */
>       unsigned int         req_flags;   /* NCSI request properties       */
>       union {
> -             unsigned char  bytes[16]; /* Command packet specific data  */
> -             unsigned short words[8];
> -             unsigned int   dwords[4];
> +#define NCSI_MAX_DATA_BYTES 16
> +             unsigned char  bytes[NCSI_MAX_DATA_BYTES]; /* Command packet 
> specific data  */
> +             unsigned short words[NCSI_MAX_DATA_BYTES / sizeof(unsigned 
> short)];
> +             unsigned int   dwords[NCSI_MAX_DATA_BYTES / sizeof(unsigned 
> int)];
>       };
>       unsigned char        *data;       /* NCSI OEM data                 */
>       struct genl_info     *info;       /* Netlink information           */
> diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c index 
> 8b386d766e7d..7d2a43f30eb1 100644
> --- a/net/ncsi/ncsi-netlink.c
> +++ b/net/ncsi/ncsi-netlink.c
> @@ -459,6 +459,15 @@ static int ncsi_send_cmd_nl(struct sk_buff *msg, struct 
> genl_info *info)
>       nca.payload = ntohs(hdr->length);
>       nca.data = data + sizeof(*hdr);
>  
> +     if (nca.payload <= NCSI_MAX_DATA_BYTES) {
> +             memcpy(nca.bytes, nca.data, nca.payload);
> +     } else {
> +             netdev_info(ndp->ndev.dev, "NCSI:payload size %u exceeds max 
> %u\n",
> +                         nca.payload, NCSI_MAX_DATA_BYTES);
> +             ret = -EINVAL;
> +             goto out_netlink;
> +     }
> +
>       ret = ncsi_xmit_cmd(&nca);
>  out_netlink:
>       if (ret != 0) {
> --
> 2.17.1