RE: [PATCH net-next v3] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
Hi Samual,
I will address the comment and send out the v4.
Thanks,
Justin
> On Mon, 2018-10-08 at 23:13 +, justin.l...@dell.com wrote:
> > The new command (NCSI_CMD_SEND_CMD) is added to allow user space
> > application to send NC-SI command to the network card.
> > Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and
> > response.
> >
> > The work flow is as below.
> >
> > Request:
> > User space application
> > -> Netlink interface (msg)
> > -> new Netlink handler - ncsi_send_cmd_nl()
> > -> ncsi_xmit_cmd()
> >
> > Response:
> > Response received - ncsi_rcv_rsp()
> > -> internal response handler - ncsi_rsp_handler_xxx()
> > -> ncsi_rsp_handler_netlink()
> > -> ncsi_send_netlink_rsp ()
> > -> Netlink interface (msg)
> > -> user space application
> >
> > Command timeout - ncsi_request_timeout()
> > -> ncsi_send_netlink_timeout ()
> > -> Netlink interface (msg with zero data length)
> > -> user space application
> >
> > Error:
> > Error detected
> > -> ncsi_send_netlink_err ()
> > -> Netlink interface (err msg)
> > -> user space application
>
> Hi Justin,
>
> I've built and tested this and it works as expected; except for some very
> minor comments below:
>
> Reviewed-by: Samuel Mendoza-Jonas
>
> >
> > V3: Based on http://patchwork.ozlabs.org/patch/979688/ to remove the
> > duplicated code.
> > V2: Remove non-related debug message and clean up the code.
>
> It's better to put these change notes under the --- below so they're not
> included in the commit message, but thanks for including them!
>
> >
> >
> > Signed-off-by: Justin Lee
> >
> >
> > ---
> > include/uapi/linux/ncsi.h | 3 +
> > net/ncsi/internal.h | 10 ++-
> > net/ncsi/ncsi-cmd.c | 8 ++
> > net/ncsi/ncsi-manage.c| 16
> > net/ncsi/ncsi-netlink.c | 204
> > ++
> > net/ncsi/ncsi-netlink.h | 12 +++
> > net/ncsi/ncsi-rsp.c | 67 +--
> > 7 files changed, 314 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> > index 4c292ec..4992bfc 100644
> > --- a/include/uapi/linux/ncsi.h
> > +++ b/include/uapi/linux/ncsi.h
> > @@ -30,6 +30,7 @@ enum ncsi_nl_commands {
> > NCSI_CMD_PKG_INFO,
> > NCSI_CMD_SET_INTERFACE,
> > NCSI_CMD_CLEAR_INTERFACE,
> > + NCSI_CMD_SEND_CMD,
> >
> > __NCSI_CMD_AFTER_LAST,
> > NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
> > @@ -43,6 +44,7 @@ enum ncsi_nl_commands {
> > * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
> > * @NCSI_ATTR_PACKAGE_ID: package ID
> > * @NCSI_ATTR_CHANNEL_ID: channel ID
> > + * @NCSI_ATTR_DATA: command payload
> > * @NCSI_ATTR_MAX: highest attribute number
> > */
> > enum ncsi_nl_attrs {
> > @@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
> > NCSI_ATTR_PACKAGE_LIST,
> > NCSI_ATTR_PACKAGE_ID,
> > NCSI_ATTR_CHANNEL_ID,
> > + NCSI_ATTR_DATA,
> >
> > __NCSI_ATTR_AFTER_LAST,
> > NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
> > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> > index 3d0a33b..e9db100 100644
> > --- a/net/ncsi/internal.h
> > +++ b/net/ncsi/internal.h
> > @@ -175,6 +175,8 @@ struct ncsi_package;
> > #define NCSI_RESERVED_CHANNEL 0x1f
> > #define NCSI_CHANNEL_INDEX(c) ((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
> > #define NCSI_TO_CHANNEL(p, c) (((p) << NCSI_PACKAGE_SHIFT) | (c))
> > +#define NCSI_MAX_PACKAGE 8
> > +#define NCSI_MAX_CHANNEL 32
> >
> > struct ncsi_channel {
> > unsigned char id;
> > @@ -219,12 +221,17 @@ struct ncsi_request {
> > unsigned charid; /* Request ID - 0 to 255 */
> > bool used;/* Request that has been assigned */
> > unsigned int flags; /* NCSI request property */
> > -#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> > +#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> > +#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
> > struct ncsi_dev_priv *ndp;/* Associated NCSI device */
> > struct sk_buff *cmd;/* Associated NCSI command packet */
> > struct sk_buff *rsp;/* Associated NCSI response packet */
> > struct timer_listtimer; /* Timer on waiting for response */
> > bool enabled; /* Time has been enabled or not*/
> > +
> > + u32 snd_seq; /* netlink sending sequence number */
> > + u32 snd_portid; /* netlink portid of sender*/
> > + struct nlmsghdr nlhdr; /* netlink message header */
> > };
> >
> > enum {
> > @@ -310,6 +317,7 @@ struct ncsi_cmd_arg {
> > unsigned int dwords[4];
> > };
> > unsigned char*data; /* NCSI OEM data */
> > + struct genl_info *info; /* Netlink information */
> > };
> >
> > extern struct
Re: [PATCH net-next v3] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
On Mon, 2018-10-08 at 23:13 +, justin.l...@dell.com wrote:
> The new command (NCSI_CMD_SEND_CMD) is added to allow user space
> application to send NC-SI command to the network card.
> Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and
> response.
>
> The work flow is as below.
>
> Request:
> User space application
> -> Netlink interface (msg)
> -> new Netlink handler - ncsi_send_cmd_nl()
> -> ncsi_xmit_cmd()
>
> Response:
> Response received - ncsi_rcv_rsp()
> -> internal response handler - ncsi_rsp_handler_xxx()
> -> ncsi_rsp_handler_netlink()
> -> ncsi_send_netlink_rsp ()
> -> Netlink interface (msg)
> -> user space application
>
> Command timeout - ncsi_request_timeout()
> -> ncsi_send_netlink_timeout ()
> -> Netlink interface (msg with zero data length)
> -> user space application
>
> Error:
> Error detected
> -> ncsi_send_netlink_err ()
> -> Netlink interface (err msg)
> -> user space application
Hi Justin,
I've built and tested this and it works as expected; except for some very
minor comments below:
Reviewed-by: Samuel Mendoza-Jonas
>
> V3: Based on http://patchwork.ozlabs.org/patch/979688/ to remove the
> duplicated code.
> V2: Remove non-related debug message and clean up the code.
It's better to put these change notes under the --- below so they're not
included in the commit message, but thanks for including them!
>
>
> Signed-off-by: Justin Lee
>
>
> ---
> include/uapi/linux/ncsi.h | 3 +
> net/ncsi/internal.h | 10 ++-
> net/ncsi/ncsi-cmd.c | 8 ++
> net/ncsi/ncsi-manage.c| 16
> net/ncsi/ncsi-netlink.c | 204
> ++
> net/ncsi/ncsi-netlink.h | 12 +++
> net/ncsi/ncsi-rsp.c | 67 +--
> 7 files changed, 314 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> index 4c292ec..4992bfc 100644
> --- a/include/uapi/linux/ncsi.h
> +++ b/include/uapi/linux/ncsi.h
> @@ -30,6 +30,7 @@ enum ncsi_nl_commands {
> NCSI_CMD_PKG_INFO,
> NCSI_CMD_SET_INTERFACE,
> NCSI_CMD_CLEAR_INTERFACE,
> + NCSI_CMD_SEND_CMD,
>
> __NCSI_CMD_AFTER_LAST,
> NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
> @@ -43,6 +44,7 @@ enum ncsi_nl_commands {
> * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
> * @NCSI_ATTR_PACKAGE_ID: package ID
> * @NCSI_ATTR_CHANNEL_ID: channel ID
> + * @NCSI_ATTR_DATA: command payload
> * @NCSI_ATTR_MAX: highest attribute number
> */
> enum ncsi_nl_attrs {
> @@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
> NCSI_ATTR_PACKAGE_LIST,
> NCSI_ATTR_PACKAGE_ID,
> NCSI_ATTR_CHANNEL_ID,
> + NCSI_ATTR_DATA,
>
> __NCSI_ATTR_AFTER_LAST,
> NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 3d0a33b..e9db100 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -175,6 +175,8 @@ struct ncsi_package;
> #define NCSI_RESERVED_CHANNEL0x1f
> #define NCSI_CHANNEL_INDEX(c)((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
> #define NCSI_TO_CHANNEL(p, c)(((p) << NCSI_PACKAGE_SHIFT) | (c))
> +#define NCSI_MAX_PACKAGE 8
> +#define NCSI_MAX_CHANNEL 32
>
> struct ncsi_channel {
> unsigned char id;
> @@ -219,12 +221,17 @@ struct ncsi_request {
> unsigned charid; /* Request ID - 0 to 255 */
> bool used;/* Request that has been assigned */
> unsigned int flags; /* NCSI request property */
> -#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
> struct ncsi_dev_priv *ndp;/* Associated NCSI device */
> struct sk_buff *cmd;/* Associated NCSI command packet */
> struct sk_buff *rsp;/* Associated NCSI response packet */
> struct timer_listtimer; /* Timer on waiting for response */
> bool enabled; /* Time has been enabled or not*/
> +
> + u32 snd_seq; /* netlink sending sequence number */
> + u32 snd_portid; /* netlink portid of sender*/
> + struct nlmsghdr nlhdr; /* netlink message header */
> };
>
> enum {
> @@ -310,6 +317,7 @@ struct ncsi_cmd_arg {
> unsigned int dwords[4];
> };
> unsigned char*data; /* NCSI OEM data */
> + struct genl_info *info; /* Netlink information */
> };
>
> extern struct list_head ncsi_dev_list;
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index 82b7d92..356af47 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
>
> #include
[PATCH net-next v3] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
The new command (NCSI_CMD_SEND_CMD) is added to allow user space
application to send NC-SI command to the network card.
Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and
response.
The work flow is as below.
Request:
User space application
-> Netlink interface (msg)
-> new Netlink handler - ncsi_send_cmd_nl()
-> ncsi_xmit_cmd()
Response:
Response received - ncsi_rcv_rsp()
-> internal response handler - ncsi_rsp_handler_xxx()
-> ncsi_rsp_handler_netlink()
-> ncsi_send_netlink_rsp ()
-> Netlink interface (msg)
-> user space application
Command timeout - ncsi_request_timeout()
-> ncsi_send_netlink_timeout ()
-> Netlink interface (msg with zero data length)
-> user space application
Error:
Error detected
-> ncsi_send_netlink_err ()
-> Netlink interface (err msg)
-> user space application
V3: Based on http://patchwork.ozlabs.org/patch/979688/ to remove the duplicated
code.
V2: Remove non-related debug message and clean up the code.
Signed-off-by: Justin Lee
---
include/uapi/linux/ncsi.h | 3 +
net/ncsi/internal.h | 10 ++-
net/ncsi/ncsi-cmd.c | 8 ++
net/ncsi/ncsi-manage.c| 16
net/ncsi/ncsi-netlink.c | 204 ++
net/ncsi/ncsi-netlink.h | 12 +++
net/ncsi/ncsi-rsp.c | 67 +--
7 files changed, 314 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
index 4c292ec..4992bfc 100644
--- a/include/uapi/linux/ncsi.h
+++ b/include/uapi/linux/ncsi.h
@@ -30,6 +30,7 @@ enum ncsi_nl_commands {
NCSI_CMD_PKG_INFO,
NCSI_CMD_SET_INTERFACE,
NCSI_CMD_CLEAR_INTERFACE,
+ NCSI_CMD_SEND_CMD,
__NCSI_CMD_AFTER_LAST,
NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
@@ -43,6 +44,7 @@ enum ncsi_nl_commands {
* @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
* @NCSI_ATTR_PACKAGE_ID: package ID
* @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_DATA: command payload
* @NCSI_ATTR_MAX: highest attribute number
*/
enum ncsi_nl_attrs {
@@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
NCSI_ATTR_PACKAGE_LIST,
NCSI_ATTR_PACKAGE_ID,
NCSI_ATTR_CHANNEL_ID,
+ NCSI_ATTR_DATA,
__NCSI_ATTR_AFTER_LAST,
NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 3d0a33b..e9db100 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -175,6 +175,8 @@ struct ncsi_package;
#define NCSI_RESERVED_CHANNEL 0x1f
#define NCSI_CHANNEL_INDEX(c) ((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
#define NCSI_TO_CHANNEL(p, c) (((p) << NCSI_PACKAGE_SHIFT) | (c))
+#define NCSI_MAX_PACKAGE 8
+#define NCSI_MAX_CHANNEL 32
struct ncsi_channel {
unsigned char id;
@@ -219,12 +221,17 @@ struct ncsi_request {
unsigned charid; /* Request ID - 0 to 255 */
bool used;/* Request that has been assigned */
unsigned int flags; /* NCSI request property */
-#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
+#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
struct ncsi_dev_priv *ndp;/* Associated NCSI device */
struct sk_buff *cmd;/* Associated NCSI command packet */
struct sk_buff *rsp;/* Associated NCSI response packet */
struct timer_listtimer; /* Timer on waiting for response */
bool enabled; /* Time has been enabled or not*/
+
+ u32 snd_seq; /* netlink sending sequence number */
+ u32 snd_portid; /* netlink portid of sender*/
+ struct nlmsghdr nlhdr; /* netlink message header */
};
enum {
@@ -310,6 +317,7 @@ struct ncsi_cmd_arg {
unsigned int dwords[4];
};
unsigned char*data; /* NCSI OEM data */
+ struct genl_info *info; /* Netlink information */
};
extern struct list_head ncsi_dev_list;
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 82b7d92..356af47 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include "internal.h"
#include "ncsi-pkt.h"
@@ -346,6 +347,13 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
if (!nr)
return -ENOMEM;
+ /* track netlink information */
+ if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+ nr->snd_seq = nca->info->snd_seq;
+ nr->snd_portid = nca->info->snd_portid;
+ nr->nlhdr = *nca->info->nlhdr;
+ }
+
/* Prepare the packet */
nca->id = nr->id;
ret = nch->handler(nr->cmd, nca);
diff --git
