Linux-Aspeed Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i2c-next v7 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
From: Jae Hyun Yoo @ 2018-10-05 21:45 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181005214507.26315-1-jae.hyun.yoo@linux.intel.com>

This commit removes hard-coded bus timeout value setting so that
it can be set by i2c-core-base.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 drivers/i2c/busses/i2c-aspeed.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 8dc9161ced38..833b6b6a4c7e 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -930,7 +930,6 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	init_completion(&bus->cmd_complete);
 	bus->adap.owner = THIS_MODULE;
 	bus->adap.retries = 0;
-	bus->adap.timeout = 5 * HZ;
 	bus->adap.algo = &aspeed_i2c_algo;
 	bus->adap.dev.parent = &pdev->dev;
 	bus->adap.dev.of_node = pdev->dev.of_node;
-- 
2.19.0


^ permalink raw reply related

* [PATCH i2c-next v7 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: Jae Hyun Yoo @ 2018-10-05 21:45 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181005214507.26315-1-jae.hyun.yoo@linux.intel.com>

In multi-master environment, this driver's master cannot know
exactly when peer master sends data to this driver's slave so a
case can be happened that this master tries to send data through
the master_xfer function but slave data from peer master is still
being processed by this driver.

To prevent any state corruption in the case, this patch adds
checking code if any slave operation is ongoing and it waits up to
the bus timeout duration before starting a master_xfer operation.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 53 +++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 833b6b6a4c7e..8d60d7e5b323 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -12,6 +12,7 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
@@ -115,6 +116,9 @@
 /* 0x18 : I2CD Slave Device Address Register   */
 #define ASPEED_I2CD_DEV_ADDR_MASK			GENMASK(6, 0)
 
+/* Busy checking */
+#define ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US		(10 * 1000)
+
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
 	ASPEED_I2C_MASTER_START,
@@ -156,6 +160,8 @@ struct aspeed_i2c_bus {
 	int				cmd_err;
 	/* Protected only by i2c_lock_bus */
 	int				master_xfer_result;
+	/* Multi-master */
+	bool				multi_master;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	struct i2c_client		*slave;
 	enum aspeed_i2c_slave_state	slave_state;
@@ -596,27 +602,42 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 	return irq_remaining ? IRQ_NONE : IRQ_HANDLED;
 }
 
+static int aspeed_i2c_check_bus_busy(struct aspeed_i2c_bus *bus)
+{
+	ktime_t timeout;
+
+	if (bus->multi_master) {
+		might_sleep();
+		timeout = ktime_add_ms(ktime_get(),
+				       jiffies_to_msecs(bus->adap.timeout));
+	}
+
+	for (;;) {
+		if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+		      ASPEED_I2CD_BUS_BUSY_STS) &&
+		    bus->slave_state == ASPEED_I2C_SLAVE_STOP)
+			return 0;
+		if (!bus->multi_master)
+			break;
+		if (ktime_compare(ktime_get(), timeout) > 0)
+			break;
+		usleep_range((ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US >> 2) + 1,
+			     ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US);
+	}
+
+	return aspeed_i2c_recover_bus(bus);
+}
+
 static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
 				  struct i2c_msg *msgs, int num)
 {
 	struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
 	unsigned long time_left, flags;
-	int ret = 0;
 
-	spin_lock_irqsave(&bus->lock, flags);
-	bus->cmd_err = 0;
-
-	/* If bus is busy, attempt recovery. We assume a single master
-	 * environment.
-	 */
-	if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
-		spin_unlock_irqrestore(&bus->lock, flags);
-		ret = aspeed_i2c_recover_bus(bus);
-		if (ret)
-			return ret;
-		spin_lock_irqsave(&bus->lock, flags);
-	}
+	if (aspeed_i2c_check_bus_busy(bus))
+		return -EAGAIN;
 
+	spin_lock_irqsave(&bus->lock, flags);
 	bus->cmd_err = 0;
 	bus->msgs = msgs;
 	bus->msgs_index = 0;
@@ -827,7 +848,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
 	if (ret < 0)
 		return ret;
 
-	if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
+	if (of_property_read_bool(pdev->dev.of_node, "multi-master"))
+		bus->multi_master = true;
+	else
 		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
 
 	/* Enable Master Mode */
-- 
2.19.0


^ permalink raw reply related

* [PATCH net-next] net/ncsi: Add NCSI OEM command support
From: David Miller @ 2018-10-05 21:54 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181005174602.828803-1-vijaykhemka@fb.com>

From: Vijay Khemka <vijaykhemka@fb.com>
Date: Fri, 5 Oct 2018 10:46:01 -0700

> This patch adds OEM commands and response handling. It also defines OEM
> command and response structure as per NCSI specification along with its
> handlers.
> 
> ncsi_cmd_handler_oem: This is a generic command request handler for OEM
> commands
> ncsi_rsp_handler_oem: This is a generic response handler for OEM commands
> 
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> Reviewed-by: Justin Lee <justin.lee1@dell.com>
> Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>

Applied.

^ permalink raw reply

* [PATCH v2] clocksource/drivers/fttmr010: fix invalid interrupt register access
From: Linus Walleij @ 2018-10-07 21:03 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181003215350.3550926-1-taoren@fb.com>

On Wed, Oct 3, 2018 at 11:54 PM Tao Ren <taoren@fb.com> wrote:

> TIMER_INTR_MASK register (Base Address of Timer + 0x38) is not designed
> for masking interrupts on ast2500 chips, and it's not even listed in
> ast2400 datasheet, so it's not safe to access TIMER_INTR_MASK on aspeed
> chips.
>
> Similarly, TIMER_INTR_STATE register (Base Address of Timer + 0x34) is
> not interrupt status register on ast2400 and ast2500 chips. Although
> there is no side effect to reset the register in fttmr010_common_init(),
> it's just misleading to do so.
>
> Besides, "count_down" is renamed to "is_aspeed" in "fttmr010" structure,
> and more comments are added so the code is more readble.
>
> Signed-off-by: Tao Ren <taoren@fb.com>
> ---
> Changes in v2:
>   - "count_down" is renamed to "is_aspeed" in "fttmr010" structure.
>   - more comments are added to make the code more readable.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH net-next 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Samuel Mendoza-Jonas @ 2018-10-08  0:57 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181005190126.983734-1-vijaykhemka@fb.com>

On Fri, 2018-10-05 at 12:01 -0700, Vijay Khemka wrote:
> This patch adds OEM Broadcom commands and response handling. It also
> defines OEM Get MAC Address handler to get and configure the device.
> 
> ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> getting mac address.
> ncsi_rsp_handler_oem_bcm: This handles response received for all
> broadcom OEM commands.
> ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> set it to device.
> 
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>

Hi Vijay,

Looks good, and I've tested this on a BMC with a Broadcom network
controller and it properly sets a MAC address. A question below about the
response handler:

> ---
>  net/ncsi/Kconfig       |  6 ++++
>  net/ncsi/internal.h    |  8 +++++
>  net/ncsi/ncsi-manage.c | 70 +++++++++++++++++++++++++++++++++++++++++-
>  net/ncsi/ncsi-pkt.h    |  8 +++++
>  net/ncsi/ncsi-rsp.c    | 42 ++++++++++++++++++++++++-
>  5 files changed, 132 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> index 08a8a6031fd7..7f2b46108a24 100644
> --- a/net/ncsi/Kconfig
> +++ b/net/ncsi/Kconfig
> @@ -10,3 +10,9 @@ config NET_NCSI
>  	  support. Enable this only if your system connects to a network
>  	  device via NCSI and the ethernet driver you're using supports
>  	  the protocol explicitly.
> +config NCSI_OEM_CMD_GET_MAC
> +	bool "Get NCSI OEM MAC Address"
> +	depends on NET_NCSI
> +	---help---
> +	  This allows to get MAC address from NCSI firmware and set them back to
> +		controller.
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 3d0a33b874f5..45883b32790e 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -71,6 +71,13 @@ enum {
>  /* OEM Vendor Manufacture ID */
>  #define NCSI_OEM_MFR_MLX_ID             0x8119
>  #define NCSI_OEM_MFR_BCM_ID             0x113d
> +/* Broadcom specific OEM Command */
> +#define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC */
> +/* OEM Command payload lengths*/
> +#define NCSI_OEM_BCM_CMD_GMA_LEN        12
> +/* Mac address offset in OEM response */
> +#define BCM_MAC_ADDR_OFFSET             28
> +
>  
>  struct ncsi_channel_version {
>  	u32 version;		/* Supported BCD encoded NCSI version */
> @@ -240,6 +247,7 @@ enum {
>  	ncsi_dev_state_probe_dp,
>  	ncsi_dev_state_config_sp	= 0x0301,
>  	ncsi_dev_state_config_cis,
> +	ncsi_dev_state_config_oem_gma,
>  	ncsi_dev_state_config_clear_vids,
>  	ncsi_dev_state_config_svf,
>  	ncsi_dev_state_config_ev,
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 091284760d21..e5bfd9245b5d 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -635,6 +635,39 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +
> +/* NCSI OEM Command APIs */
> +static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
> +{
> +	int ret = 0;
> +	unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
> +
> +	nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
> +
> +	memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
> +	*(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
> +	data[5] = NCSI_OEM_BCM_CMD_GMA;
> +
> +	nca->data = data;
> +
> +	ret = ncsi_xmit_cmd(nca);
> +	if (ret)
> +		netdev_err(nca->ndp->ndev.dev,
> +			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
> +			   nca->type);
> +}
> +
> +/* OEM Command handlers initialization */
> +static struct ncsi_oem_gma_handler {
> +	unsigned int	mfr_id;
> +	void		(*handler)(struct ncsi_cmd_arg *nca);
> +} ncsi_oem_gma_handlers[] = {
> +	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
> +};
> +
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
>  static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
>  {
>  	struct ncsi_dev *nd = &ndp->ndev;
> @@ -643,9 +676,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
>  	struct ncsi_channel *nc = ndp->active_channel;
>  	struct ncsi_channel *hot_nc = NULL;
>  	struct ncsi_cmd_arg nca;
> +	struct ncsi_oem_gma_handler *nch = NULL;
>  	unsigned char index;
>  	unsigned long flags;
> -	int ret;
> +	int ret, i;
>  
>  	nca.ndp = ndp;
>  	nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> @@ -685,6 +719,40 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
>  			goto error;
>  		}
>  
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +		nd->state = ncsi_dev_state_config_oem_gma;
> +		break;
> +	case ncsi_dev_state_config_oem_gma:
> +		nca.type = NCSI_PKT_CMD_OEM;
> +		nca.package = np->id;
> +		nca.channel = nc->id;
> +		ndp->pending_req_num = 1;
> +
> +		/* Check for manufacturer id and Find the handler */
> +		for (i = 0; i < ARRAY_SIZE(ncsi_oem_gma_handlers); i++) {
> +			if (ncsi_oem_gma_handlers[i].mfr_id ==
> +					nc->version.mf_id) {
> +				if (ncsi_oem_gma_handlers[i].handler)
> +					nch = &ncsi_oem_gma_handlers[i];
> +				else
> +					nch = NULL;
> +
> +				break;
> +			}
> +		}
> +
> +		if (!nch) {
> +			netdev_err(ndp->ndev.dev, "No handler available for GMA with MFR-ID (0x%x)\n",
> +				   nc->version.mf_id);
> +			nd->state = ncsi_dev_state_config_clear_vids;
> +			schedule_work(&ndp->work);
> +			break;
> +		}
> +
> +		/* Get Mac address from NCSI device */
> +		nch->handler(&nca);
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
>  		nd->state = ncsi_dev_state_config_clear_vids;
>  		break;
>  	case ncsi_dev_state_config_clear_vids:
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 0f2087c8d42a..4d3f06be38bd 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -165,6 +165,14 @@ struct ncsi_rsp_oem_pkt {
>  	unsigned char           data[];      /* Payload data      */
>  };
>  
> +/* Broadcom Response Data */
> +struct ncsi_rsp_oem_bcm_pkt {
> +	unsigned char           ver;         /* Payload Version   */
> +	unsigned char           type;        /* OEM Command type  */
> +	__be16                  len;         /* Payload Length    */
> +	unsigned char           data[];      /* Cmd specific Data */
> +};
> +
>  /* Get Link Status */
>  struct ncsi_rsp_gls_pkt {
>  	struct ncsi_rsp_pkt_hdr rsp;        /* Response header   */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index d66b34749027..bc20f7036579 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -596,12 +596,52 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
>  	return 0;
>  }
>  
> +/* Response handler for Broadcom command Get Mac Address */
> +static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
> +{
> +	struct ncsi_rsp_oem_pkt *rsp;
> +	struct ncsi_dev_priv *ndp = nr->ndp;
> +	struct net_device *ndev = ndp->ndev.dev;
> +	int ret = 0;
> +	const struct net_device_ops *ops = ndev->netdev_ops;
> +	struct sockaddr saddr;
> +
> +	/* Get the response header */
> +	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> +
> +	saddr.sa_family = ndev->type;
> +	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> +	memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
> +	/* Increase mac address by 1 for BMC's address */
> +	saddr.sa_data[ETH_ALEN - 1]++;

Is this convention documented somewhere, and could you provide a link to
it if so?

Also what happens here if the final byte of the address is 0xff, this
will overflow and not carry right?

> +	ret = ops->ndo_set_mac_address(ndev, &saddr);
> +	if (ret < 0)
> +		netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");

Also a minor nitpick, there's a bonus "'" in this message.

> +
> +	return ret;
> +}
> +
> +/* Response handler for Broadcom card */
> +static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
> +{
> +	struct ncsi_rsp_oem_pkt *rsp;
> +	struct ncsi_rsp_oem_bcm_pkt *bcm;
> +
> +	/* Get the response header */
> +	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> +	bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
> +
> +	if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
> +		return ncsi_rsp_handler_oem_bcm_gma(nr);
> +	return 0;
> +}
> +
>  static struct ncsi_rsp_oem_handler {
>  	unsigned int	mfr_id;
>  	int		(*handler)(struct ncsi_request *nr);
>  } ncsi_rsp_oem_handlers[] = {
>  	{ NCSI_OEM_MFR_MLX_ID, NULL },
> -	{ NCSI_OEM_MFR_BCM_ID, NULL }
> +	{ NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
>  };
>  
>  /* Response handler for OEM command */



^ permalink raw reply

* [PATCH net-next 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-08 19:18 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <ee398ff59257b33e64ba7b641bf29096972a5006.camel@mendozajonas.com>



?On 10/7/18, 5:58 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote:

    On Fri, 2018-10-05 at 12:01 -0700, Vijay Khemka wrote:
    > This patch adds OEM Broadcom commands and response handling. It also
    > defines OEM Get MAC Address handler to get and configure the device.
    > 
    > ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
    > getting mac address.
    > ncsi_rsp_handler_oem_bcm: This handles response received for all
    > broadcom OEM commands.
    > ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
    > set it to device.
    > 
    > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
    
    Hi Vijay,
    
    Looks good, and I've tested this on a BMC with a Broadcom network
    controller and it properly sets a MAC address. A question below about the
    response handler:

  Hi Sam,
  Thanks for feedback, I will update code for response handler. Please see my response below.
    

    > +/* Response handler for Broadcom command Get Mac Address */
    > +static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
    > +{
    > +	struct ncsi_rsp_oem_pkt *rsp;
    > +	struct ncsi_dev_priv *ndp = nr->ndp;
    > +	struct net_device *ndev = ndp->ndev.dev;
    > +	int ret = 0;
    > +	const struct net_device_ops *ops = ndev->netdev_ops;
    > +	struct sockaddr saddr;
    > +
    > +	/* Get the response header */
    > +	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
    > +
    > +	saddr.sa_family = ndev->type;
    > +	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
    > +	memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
    > +	/* Increase mac address by 1 for BMC's address */
    > +	saddr.sa_data[ETH_ALEN - 1]++;
    
    Is this convention documented somewhere, and could you provide a link to
    it if so?
    
    Also what happens here if the final byte of the address is 0xff, this
    will overflow and not carry right?

  This is a Facebook requirement to increment MAC address by 1 as base address is defined for 
  server followed by BMC address. I will handle this Facebook specific code differently in new 
  patch. For generic Broadcom card, I will just get base mac address and not add any convention.
    
    > +	ret = ops->ndo_set_mac_address(ndev, &saddr);
    > +	if (ret < 0)
    > +		netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
    
    Also a minor nitpick, there's a bonus "'" in this message.
    
  Will be taken care in next version.
    


^ permalink raw reply

* [PATCH net-next v3] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-08 23:13 UTC (permalink / raw)
  To: linux-aspeed

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 <justin.lee1@dell.com> 


---
 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 char        id;      /* 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_list    timer;   /* 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 <net/ncsi.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
+#include <net/genetlink.h>
 
 #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 a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0912847..76a4bcb 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -19,6 +19,7 @@
 #include <net/addrconf.h>
 #include <net/ipv6.h>
 #include <net/if_inet6.h>
+#include <net/genetlink.h>
 
 #include "internal.h"
 #include "ncsi-pkt.h"
@@ -406,6 +407,9 @@ static void ncsi_request_timeout(struct timer_list *t)
 {
 	struct ncsi_request *nr = from_timer(nr, t, timer);
 	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct ncsi_package *np;
+	struct ncsi_channel *nc;
+	struct ncsi_cmd_pkt *cmd;
 	unsigned long flags;
 
 	/* If the request already had associated response,
@@ -419,6 +423,18 @@ static void ncsi_request_timeout(struct timer_list *t)
 	}
 	spin_unlock_irqrestore(&ndp->lock, flags);
 
+	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+		if (nr->cmd) {
+			/* Find the package */
+			cmd = (struct ncsi_cmd_pkt *)
+			      skb_network_header(nr->cmd);
+			ncsi_find_package_and_channel(ndp,
+						      cmd->cmd.common.channel,
+						      &np, &nc);
+			ncsi_send_netlink_timeout(nr, np, nc);
+		}
+	}
+
 	/* Release the request */
 	ncsi_free_request(nr);
 }
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 45f33d6..3941bf6 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -20,6 +20,7 @@
 #include <uapi/linux/ncsi.h>
 
 #include "internal.h"
+#include "ncsi-pkt.h"
 #include "ncsi-netlink.h"
 
 static struct genl_family ncsi_genl_family;
@@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
 	[NCSI_ATTR_PACKAGE_LIST] =	{ .type = NLA_NESTED },
 	[NCSI_ATTR_PACKAGE_ID] =	{ .type = NLA_U32 },
 	[NCSI_ATTR_CHANNEL_ID] =	{ .type = NLA_U32 },
+	[NCSI_ATTR_DATA] =		{ .type = NLA_BINARY, .len = 2048 },
 };
 
 static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
@@ -366,6 +368,202 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
 	return 0;
 }
 
+static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	struct ncsi_dev_priv *ndp;
+
+	struct ncsi_cmd_arg nca;
+	struct ncsi_pkt_hdr *hdr;
+
+	u32 package_id, channel_id;
+	unsigned char *data;
+	int len, ret;
+
+	if (!info || !info->attrs) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_IFINDEX]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
+			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+	if (!ndp) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
+	channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
+
+	if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
+		ret = -ERANGE;
+		goto out_netlink;
+	}
+
+	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
+	if (len < sizeof(struct ncsi_pkt_hdr)) {
+		netdev_info(ndp->ndev.dev, "NCSI: no OEM command to send %u\n",
+			    package_id);
+		ret = -EINVAL;
+		goto out_netlink;
+	} else {
+		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
+	}
+
+	hdr = (struct ncsi_pkt_hdr *)data;
+
+	nca.ndp = ndp;
+	nca.package = (unsigned char)package_id;
+	nca.channel = (unsigned char)channel_id;
+	nca.type = hdr->type;
+	nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
+	nca.info = info;
+	nca.payload = ntohs(hdr->length);
+	nca.data = data + sizeof(*hdr);
+
+	ret = ncsi_xmit_cmd(&nca);
+out_netlink:
+	if (ret != 0) {
+		netdev_err(ndp->ndev.dev,
+			   "NCSI: Error %d sending OEM command\n",
+			   ret);
+		ncsi_send_netlink_err(ndp->ndev.dev,
+				      info->snd_seq,
+				      info->snd_portid,
+				      info->nlhdr,
+				      ret);
+	}
+out:
+	return ret;
+}
+
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+			  struct ncsi_package *np,
+			  struct ncsi_channel *nc)
+{
+	struct sk_buff *skb;
+	struct net *net;
+	void *hdr;
+	int rc;
+
+	netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
+
+	net = dev_net(nr->rsp->dev);
+
+	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+	if (!hdr) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
+	if (np)
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+	if (nc)
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+	rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
+	if (rc)
+		goto err;
+
+	genlmsg_end(skb, hdr);
+	return genlmsg_unicast(net, skb, nr->snd_portid);
+
+err:
+	kfree_skb(skb);
+	return rc;
+}
+
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+			      struct ncsi_package *np,
+			      struct ncsi_channel *nc)
+{
+	struct sk_buff *skb;
+	struct net *net;
+	void *hdr;
+
+	netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
+
+	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+	if (!hdr) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	net = dev_net(nr->cmd->dev);
+
+	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
+
+	if (np)
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
+			    NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
+						 nr->cmd->data)->channel)));
+
+	if (nc)
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+	genlmsg_end(skb, hdr);
+	return genlmsg_unicast(net, skb, nr->snd_portid);
+}
+
+int ncsi_send_netlink_err(struct net_device *dev,
+			  u32 snd_seq,
+			  u32 snd_portid,
+			  struct nlmsghdr *nlhdr,
+			  int err)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr *nlh;
+	struct nlmsgerr *nle;
+	struct net *net;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	net = dev_net(dev);
+
+	nlh = nlmsg_put(skb, snd_portid, snd_seq,
+			NLMSG_ERROR, sizeof(*nle), 0);
+	nle = (struct nlmsgerr *)nlmsg_data(nlh);
+	nle->error = err;
+	memcpy(&nle->msg, nlhdr, sizeof(*nlh));
+
+	nlmsg_end(skb, nlh);
+
+	return nlmsg_unicast(net->genl_sock, skb, snd_portid);
+}
+
 static const struct genl_ops ncsi_ops[] = {
 	{
 		.cmd = NCSI_CMD_PKG_INFO,
@@ -386,6 +584,12 @@ static const struct genl_ops ncsi_ops[] = {
 		.doit = ncsi_clear_interface_nl,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = NCSI_CMD_SEND_CMD,
+		.policy = ncsi_genl_policy,
+		.doit = ncsi_send_cmd_nl,
+		.flags = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family ncsi_genl_family __ro_after_init = {
diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
index 91a5c25..c4a4688 100644
--- a/net/ncsi/ncsi-netlink.h
+++ b/net/ncsi/ncsi-netlink.h
@@ -14,6 +14,18 @@
 
 #include "internal.h"
 
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+			  struct ncsi_package *np,
+			  struct ncsi_channel *nc);
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+			      struct ncsi_package *np,
+			      struct ncsi_channel *nc);
+int ncsi_send_netlink_err(struct net_device *dev,
+			  u32 snd_seq,
+			  u32 snd_portid,
+			  struct nlmsghdr *nlhdr,
+			  int err);
+
 int ncsi_init_netlink(struct net_device *dev);
 int ncsi_unregister_netlink(struct net_device *dev);
 
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b347..dd931d2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -16,9 +16,11 @@
 #include <net/ncsi.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
+#include <net/genetlink.h>
 
 #include "internal.h"
 #include "ncsi-pkt.h"
+#include "ncsi-netlink.h"
 
 static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 				 unsigned short payload)
@@ -32,15 +34,25 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 	 * before calling this function.
 	 */
 	h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
-	if (h->common.revision != NCSI_PKT_REVISION)
+
+	if (h->common.revision != NCSI_PKT_REVISION) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: unsupported header revision\n");
 		return -EINVAL;
-	if (ntohs(h->common.length) != payload)
+	}
+	if (ntohs(h->common.length) != payload) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: payload length mismatched\n");
 		return -EINVAL;
+	}
 
 	/* Check on code and reason */
 	if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
-	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
-		return -EINVAL;
+	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: non zero response/reason code\n");
+		return -EPERM;
+	}
 
 	/* Validate checksum, which might be zeroes if the
 	 * sender doesn't support checksum according to NCSI
@@ -52,8 +64,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 
 	checksum = ncsi_calculate_checksum((unsigned char *)h,
 					   sizeof(*h) + payload - 4);
-	if (*pchecksum != htonl(checksum))
+
+	if (*pchecksum != htonl(checksum)) {
+		netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
 	return 0;
 }
 
+static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_pkt *rsp;
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct ncsi_package *np;
+	struct ncsi_channel *nc;
+	int ret;
+
+	/* Find the package */
+	rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
+	ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
+				      &np, &nc);
+	if (!np)
+		return -ENODEV;
+
+	ret = ncsi_send_netlink_rsp(nr, np, nc);
+
+	return ret;
+}
+
 static struct ncsi_rsp_handler {
 	unsigned char	type;
 	int             payload;
@@ -1043,6 +1078,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 		netdev_warn(ndp->ndev.dev,
 			    "NCSI: 'bad' packet ignored for type 0x%x\n",
 			    hdr->type);
+
+		if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+			if (ret == -EPERM)
+				goto out_netlink;
+			else
+				ncsi_send_netlink_err(ndp->ndev.dev,
+						      nr->snd_seq,
+						      nr->snd_portid,
+						      &nr->nlhdr,
+						      ret);
+		}
 		goto out;
 	}
 
@@ -1052,6 +1098,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 		netdev_err(ndp->ndev.dev,
 			   "NCSI: Handler for packet type 0x%x returned %d\n",
 			   hdr->type, ret);
+
+out_netlink:
+	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+		ret = ncsi_rsp_handler_netlink(nr);
+		if (ret) {
+			netdev_err(ndp->ndev.dev,
+				   "NCSI: Netlink handler for packet type 0x%x returned %d\n",
+				   hdr->type, ret);
+		}
+	}
+
 out:
 	ncsi_free_request(nr);
 	return ret;
-- 
2.9.3


^ permalink raw reply related

* [PATCH v2 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Vijay Khemka @ 2018-10-09 17:48 UTC (permalink / raw)
  To: linux-aspeed

This patch adds OEM Broadcom commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.

ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
getting mac address.
ncsi_rsp_handler_oem_bcm: This handles response received for all
broadcom OEM commands.
ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
set it to device.

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 net/ncsi/Kconfig       |  6 ++++
 net/ncsi/internal.h    |  8 +++++
 net/ncsi/ncsi-manage.c | 70 +++++++++++++++++++++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  8 +++++
 net/ncsi/ncsi-rsp.c    | 40 +++++++++++++++++++++++-
 5 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
index 08a8a6031fd7..7f2b46108a24 100644
--- a/net/ncsi/Kconfig
+++ b/net/ncsi/Kconfig
@@ -10,3 +10,9 @@ config NET_NCSI
 	  support. Enable this only if your system connects to a network
 	  device via NCSI and the ethernet driver you're using supports
 	  the protocol explicitly.
+config NCSI_OEM_CMD_GET_MAC
+	bool "Get NCSI OEM MAC Address"
+	depends on NET_NCSI
+	---help---
+	  This allows to get MAC address from NCSI firmware and set them back to
+		controller.
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 3d0a33b874f5..45883b32790e 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -71,6 +71,13 @@ enum {
 /* OEM Vendor Manufacture ID */
 #define NCSI_OEM_MFR_MLX_ID             0x8119
 #define NCSI_OEM_MFR_BCM_ID             0x113d
+/* Broadcom specific OEM Command */
+#define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC */
+/* OEM Command payload lengths*/
+#define NCSI_OEM_BCM_CMD_GMA_LEN        12
+/* Mac address offset in OEM response */
+#define BCM_MAC_ADDR_OFFSET             28
+
 
 struct ncsi_channel_version {
 	u32 version;		/* Supported BCD encoded NCSI version */
@@ -240,6 +247,7 @@ enum {
 	ncsi_dev_state_probe_dp,
 	ncsi_dev_state_config_sp	= 0x0301,
 	ncsi_dev_state_config_cis,
+	ncsi_dev_state_config_oem_gma,
 	ncsi_dev_state_config_clear_vids,
 	ncsi_dev_state_config_svf,
 	ncsi_dev_state_config_ev,
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 091284760d21..e5bfd9245b5d 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -635,6 +635,39 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+
+/* NCSI OEM Command APIs */
+static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
+{
+	int ret = 0;
+	unsigned char data[NCSI_OEM_BCM_CMD_GMA_LEN];
+
+	nca->payload = NCSI_OEM_BCM_CMD_GMA_LEN;
+
+	memset(data, 0, NCSI_OEM_BCM_CMD_GMA_LEN);
+	*(unsigned int *)data = ntohl(NCSI_OEM_MFR_BCM_ID);
+	data[5] = NCSI_OEM_BCM_CMD_GMA;
+
+	nca->data = data;
+
+	ret = ncsi_xmit_cmd(nca);
+	if (ret)
+		netdev_err(nca->ndp->ndev.dev,
+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
+			   nca->type);
+}
+
+/* OEM Command handlers initialization */
+static struct ncsi_oem_gma_handler {
+	unsigned int	mfr_id;
+	void		(*handler)(struct ncsi_cmd_arg *nca);
+} ncsi_oem_gma_handlers[] = {
+	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+};
+
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
 static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
 {
 	struct ncsi_dev *nd = &ndp->ndev;
@@ -643,9 +676,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
 	struct ncsi_channel *nc = ndp->active_channel;
 	struct ncsi_channel *hot_nc = NULL;
 	struct ncsi_cmd_arg nca;
+	struct ncsi_oem_gma_handler *nch = NULL;
 	unsigned char index;
 	unsigned long flags;
-	int ret;
+	int ret, i;
 
 	nca.ndp = ndp;
 	nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
@@ -685,6 +719,40 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
 			goto error;
 		}
 
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+		nd->state = ncsi_dev_state_config_oem_gma;
+		break;
+	case ncsi_dev_state_config_oem_gma:
+		nca.type = NCSI_PKT_CMD_OEM;
+		nca.package = np->id;
+		nca.channel = nc->id;
+		ndp->pending_req_num = 1;
+
+		/* Check for manufacturer id and Find the handler */
+		for (i = 0; i < ARRAY_SIZE(ncsi_oem_gma_handlers); i++) {
+			if (ncsi_oem_gma_handlers[i].mfr_id ==
+					nc->version.mf_id) {
+				if (ncsi_oem_gma_handlers[i].handler)
+					nch = &ncsi_oem_gma_handlers[i];
+				else
+					nch = NULL;
+
+				break;
+			}
+		}
+
+		if (!nch) {
+			netdev_err(ndp->ndev.dev, "No handler available for GMA with MFR-ID (0x%x)\n",
+				   nc->version.mf_id);
+			nd->state = ncsi_dev_state_config_clear_vids;
+			schedule_work(&ndp->work);
+			break;
+		}
+
+		/* Get Mac address from NCSI device */
+		nch->handler(&nca);
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
 		nd->state = ncsi_dev_state_config_clear_vids;
 		break;
 	case ncsi_dev_state_config_clear_vids:
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 0f2087c8d42a..4d3f06be38bd 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,14 @@ struct ncsi_rsp_oem_pkt {
 	unsigned char           data[];      /* Payload data      */
 };
 
+/* Broadcom Response Data */
+struct ncsi_rsp_oem_bcm_pkt {
+	unsigned char           ver;         /* Payload Version   */
+	unsigned char           type;        /* OEM Command type  */
+	__be16                  len;         /* Payload Length    */
+	unsigned char           data[];      /* Cmd specific Data */
+};
+
 /* Get Link Status */
 struct ncsi_rsp_gls_pkt {
 	struct ncsi_rsp_pkt_hdr rsp;        /* Response header   */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b34749027..31672e967db2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,12 +596,50 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
 	return 0;
 }
 
+/* Response handler for Broadcom command Get Mac Address */
+static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct net_device *ndev = ndp->ndev.dev;
+	int ret = 0;
+	const struct net_device_ops *ops = ndev->netdev_ops;
+	struct sockaddr saddr;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+	saddr.sa_family = ndev->type;
+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	memcpy(saddr.sa_data, &rsp->data[BCM_MAC_ADDR_OFFSET], ETH_ALEN);
+	ret = ops->ndo_set_mac_address(ndev, &saddr);
+	if (ret < 0)
+		netdev_warn(ndev, "NCSI: Writing mac address to device failed\n");
+
+	return ret;
+}
+
+/* Response handler for Broadcom card */
+static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct ncsi_rsp_oem_bcm_pkt *bcm;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+	bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
+
+	if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
+		return ncsi_rsp_handler_oem_bcm_gma(nr);
+	return 0;
+}
+
 static struct ncsi_rsp_oem_handler {
 	unsigned int	mfr_id;
 	int		(*handler)(struct ncsi_request *nr);
 } ncsi_rsp_oem_handlers[] = {
 	{ NCSI_OEM_MFR_MLX_ID, NULL },
-	{ NCSI_OEM_MFR_BCM_ID, NULL }
+	{ NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
 };
 
 /* Response handler for OEM command */
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 2/2] net/ncsi: Add NCSI Mellanox OEM command
From: Vijay Khemka @ 2018-10-09 17:48 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181009174806.404791-1-vijaykhemka@fb.com>

This patch adds OEM Mellanox commands and response handling. It also
defines OEM Get MAC Address handler to get and configure the device.

ncsi_oem_gma_handler_mlx: This handler send NCSI mellanox command for
getting mac address.
ncsi_rsp_handler_oem_mlx: This handles response received for all
mellanox OEM commands.
ncsi_rsp_handler_oem_mlx_gma: This handles get mac address response and
set it to device.

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 net/ncsi/internal.h    |  5 +++++
 net/ncsi/ncsi-manage.c | 24 +++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  9 +++++++++
 net/ncsi/ncsi-rsp.c    | 41 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 45883b32790e..d4558628a991 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -73,10 +73,15 @@ enum {
 #define NCSI_OEM_MFR_BCM_ID             0x113d
 /* Broadcom specific OEM Command */
 #define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC */
+/* Mellanox specific OEM Command */
+#define NCSI_OEM_MLX_CMD_GMA            0x00   /* CMD ID for Get MAC */
+#define NCSI_OEM_MLX_CMD_GMA_PARAM      0x1b   /* Parameter for GMA  */
 /* OEM Command payload lengths*/
 #define NCSI_OEM_BCM_CMD_GMA_LEN        12
+#define NCSI_OEM_MLX_CMD_GMA_LEN        8
 /* Mac address offset in OEM response */
 #define BCM_MAC_ADDR_OFFSET             28
+#define MLX_MAC_ADDR_OFFSET             8
 
 
 struct ncsi_channel_version {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index e5bfd9245b5d..38aef27d2e67 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -658,12 +658,34 @@ static void ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
 			   nca->type);
 }
 
+static void ncsi_oem_gma_handler_mlx(struct ncsi_cmd_arg *nca)
+{
+	int ret = 0;
+	unsigned char data[NCSI_OEM_MLX_CMD_GMA_LEN];
+
+	nca->payload = NCSI_OEM_MLX_CMD_GMA_LEN;
+
+	memset(data, 0, NCSI_OEM_MLX_CMD_GMA_LEN);
+	*(unsigned int *)data = ntohl(NCSI_OEM_MFR_MLX_ID);
+	data[5] = NCSI_OEM_MLX_CMD_GMA;
+	data[6] = NCSI_OEM_MLX_CMD_GMA_PARAM;
+
+	nca->data = data;
+
+	ret = ncsi_xmit_cmd(nca);
+	if (ret)
+		netdev_err(nca->ndp->ndev.dev,
+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
+			   nca->type);
+}
+
 /* OEM Command handlers initialization */
 static struct ncsi_oem_gma_handler {
 	unsigned int	mfr_id;
 	void		(*handler)(struct ncsi_cmd_arg *nca);
 } ncsi_oem_gma_handlers[] = {
-	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm }
+	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
 };
 
 #endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 4d3f06be38bd..2a6d83a596c9 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -165,6 +165,15 @@ struct ncsi_rsp_oem_pkt {
 	unsigned char           data[];      /* Payload data      */
 };
 
+/* Mellanox Response Data */
+struct ncsi_rsp_oem_mlx_pkt {
+	unsigned char           cmd_rev;     /* Command Revision  */
+	unsigned char           cmd;         /* Command ID        */
+	unsigned char           param;       /* Parameter         */
+	unsigned char           optional;    /* Optional data     */
+	unsigned char           data[];      /* Data              */
+};
+
 /* Broadcom Response Data */
 struct ncsi_rsp_oem_bcm_pkt {
 	unsigned char           ver;         /* Payload Version   */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 31672e967db2..b18efa23faa2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,6 +596,45 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
 	return 0;
 }
 
+/* Response handler for Mellanox command Get Mac Address */
+static int ncsi_rsp_handler_oem_mlx_gma(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct net_device *ndev = ndp->ndev.dev;
+	int ret = 0;
+	const struct net_device_ops *ops = ndev->netdev_ops;
+	struct sockaddr saddr;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+	saddr.sa_family = ndev->type;
+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	memcpy(saddr.sa_data, &rsp->data[MLX_MAC_ADDR_OFFSET], ETH_ALEN);
+	ret = ops->ndo_set_mac_address(ndev, &saddr);
+	if (ret < 0)
+		netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
+
+	return ret;
+}
+
+/* Response handler for Mellanox card */
+static int ncsi_rsp_handler_oem_mlx(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct ncsi_rsp_oem_mlx_pkt *mlx;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+	mlx = (struct ncsi_rsp_oem_mlx_pkt *)(rsp->data);
+
+	if (mlx->cmd == NCSI_OEM_MLX_CMD_GMA &&
+	    mlx->param == NCSI_OEM_MLX_CMD_GMA_PARAM)
+		return ncsi_rsp_handler_oem_mlx_gma(nr);
+	return 0;
+}
+
 /* Response handler for Broadcom command Get Mac Address */
 static int ncsi_rsp_handler_oem_bcm_gma(struct ncsi_request *nr)
 {
@@ -638,7 +677,7 @@ static struct ncsi_rsp_oem_handler {
 	unsigned int	mfr_id;
 	int		(*handler)(struct ncsi_request *nr);
 } ncsi_rsp_oem_handlers[] = {
-	{ NCSI_OEM_MFR_MLX_ID, NULL },
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_rsp_handler_oem_mlx },
 	{ NCSI_OEM_MFR_BCM_ID, ncsi_rsp_handler_oem_bcm }
 };
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH i2c-next v7 3/5] dt-bindings: i2c: aspeed: Add 'bus-timeout-ms' property as an optional property
From: Brendan Higgins @ 2018-10-10  0:00 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181005214507.26315-4-jae.hyun.yoo@linux.intel.com>

On Fri, Oct 5, 2018 at 2:45 PM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> This commit adds 'bus-timeout-ms' property as an optional property
> which can be used for setting the bus timeout value of an adapter.
> With this patch, the bus timeout value can be set through this
> property at the probing time of this module. Still the bus timeout
> value can be set by an I2C_TIMEOUT ioctl on cdev at runtime too.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

Thanks for putting all the work in to do it this way!

^ permalink raw reply

* [PATCH i2c-next v7 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
From: Brendan Higgins @ 2018-10-10  0:00 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181005214507.26315-5-jae.hyun.yoo@linux.intel.com>

On Fri, Oct 5, 2018 at 2:45 PM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> This commit removes hard-coded bus timeout value setting so that
> it can be set by i2c-core-base.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

^ permalink raw reply

* [PATCH i2c-next v7 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: Brendan Higgins @ 2018-10-10  0:08 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181005214507.26315-6-jae.hyun.yoo@linux.intel.com>

On Fri, Oct 5, 2018 at 2:45 PM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> In multi-master environment, this driver's master cannot know
> exactly when peer master sends data to this driver's slave so a
> case can be happened that this master tries to send data through
> the master_xfer function but slave data from peer master is still
> being processed by this driver.
>
> To prevent any state corruption in the case, this patch adds
> checking code if any slave operation is ongoing and it waits up to
> the bus timeout duration before starting a master_xfer operation.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

^ permalink raw reply

* [PATCH net-next v3] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Samuel Mendoza-Jonas @ 2018-10-10  4:17 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <e32d218b9e514cc58eeb916bf0f29a52@AUSX13MPS302.AMER.DELL.COM>

On Mon, 2018-10-08 at 23:13 +0000, Justin.Lee1 at 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 <sam@mendozajonas.com>

> 
> 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 <justin.lee1@dell.com> 
> 
> 
> ---
>  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 char        id;      /* 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_list    timer;   /* 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 <net/ncsi.h>
>  #include <net/net_namespace.h>
>  #include <net/sock.h>
> +#include <net/genetlink.h>
>  
>  #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 a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 0912847..76a4bcb 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -19,6 +19,7 @@
>  #include <net/addrconf.h>
>  #include <net/ipv6.h>
>  #include <net/if_inet6.h>
> +#include <net/genetlink.h>
>  
>  #include "internal.h"
>  #include "ncsi-pkt.h"
> @@ -406,6 +407,9 @@ static void ncsi_request_timeout(struct timer_list *t)
>  {
>  	struct ncsi_request *nr = from_timer(nr, t, timer);
>  	struct ncsi_dev_priv *ndp = nr->ndp;
> +	struct ncsi_package *np;
> +	struct ncsi_channel *nc;
> +	struct ncsi_cmd_pkt *cmd;
>  	unsigned long flags;
>  
>  	/* If the request already had associated response,
> @@ -419,6 +423,18 @@ static void ncsi_request_timeout(struct timer_list *t)
>  	}
>  	spin_unlock_irqrestore(&ndp->lock, flags);
>  
> +	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> +		if (nr->cmd) {
> +			/* Find the package */
> +			cmd = (struct ncsi_cmd_pkt *)
> +			      skb_network_header(nr->cmd);
> +			ncsi_find_package_and_channel(ndp,
> +						      cmd->cmd.common.channel,
> +						      &np, &nc);
> +			ncsi_send_netlink_timeout(nr, np, nc);
> +		}
> +	}
> +
>  	/* Release the request */
>  	ncsi_free_request(nr);
>  }
> diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
> index 45f33d6..3941bf6 100644
> --- a/net/ncsi/ncsi-netlink.c
> +++ b/net/ncsi/ncsi-netlink.c
> @@ -20,6 +20,7 @@
>  #include <uapi/linux/ncsi.h>
>  
>  #include "internal.h"
> +#include "ncsi-pkt.h"
>  #include "ncsi-netlink.h"
>  
>  static struct genl_family ncsi_genl_family;
> @@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
>  	[NCSI_ATTR_PACKAGE_LIST] =	{ .type = NLA_NESTED },
>  	[NCSI_ATTR_PACKAGE_ID] =	{ .type = NLA_U32 },
>  	[NCSI_ATTR_CHANNEL_ID] =	{ .type = NLA_U32 },
> +	[NCSI_ATTR_DATA] =		{ .type = NLA_BINARY, .len = 2048 },
>  };
>  
>  static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
> @@ -366,6 +368,202 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
>  	return 0;
>  }
>  
> +static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
> +{
> +	struct ncsi_dev_priv *ndp;
> +
> +	struct ncsi_cmd_arg nca;
> +	struct ncsi_pkt_hdr *hdr;
> +
> +	u32 package_id, channel_id;
> +	unsigned char *data;
> +	int len, ret;
> +
> +	if (!info || !info->attrs) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (!info->attrs[NCSI_ATTR_IFINDEX]) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
> +			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
> +	if (!ndp) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
> +	channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
> +
> +	if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
> +		ret = -ERANGE;
> +		goto out_netlink;
> +	}
> +
> +	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
> +	if (len < sizeof(struct ncsi_pkt_hdr)) {
> +		netdev_info(ndp->ndev.dev, "NCSI: no OEM command to send %u\n",
> +			    package_id);

Technically we can send any command via this interface so saying "no OEM
command" may be a little confusing as a message.

> +		ret = -EINVAL;
> +		goto out_netlink;
> +	} else {
> +		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
> +	}
> +
> +	hdr = (struct ncsi_pkt_hdr *)data;
> +
> +	nca.ndp = ndp;
> +	nca.package = (unsigned char)package_id;
> +	nca.channel = (unsigned char)channel_id;
> +	nca.type = hdr->type;
> +	nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
> +	nca.info = info;
> +	nca.payload = ntohs(hdr->length);
> +	nca.data = data + sizeof(*hdr);
> +
> +	ret = ncsi_xmit_cmd(&nca);
> +out_netlink:
> +	if (ret != 0) {
> +		netdev_err(ndp->ndev.dev,
> +			   "NCSI: Error %d sending OEM command\n",
> +			   ret);
> +		ncsi_send_netlink_err(ndp->ndev.dev,
> +				      info->snd_seq,
> +				      info->snd_portid,
> +				      info->nlhdr,
> +				      ret);
> +	}
> +out:
> +	return ret;
> +}
> +
> +int ncsi_send_netlink_rsp(struct ncsi_request *nr,
> +			  struct ncsi_package *np,
> +			  struct ncsi_channel *nc)
> +{
> +	struct sk_buff *skb;
> +	struct net *net;
> +	void *hdr;
> +	int rc;
> +
> +	netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);

We probably don't need this message.

> +
> +	net = dev_net(nr->rsp->dev);
> +
> +	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
> +			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
> +	if (!hdr) {
> +		kfree_skb(skb);
> +		return -EMSGSIZE;
> +	}
> +
> +	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
> +	if (np)
> +		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
> +	if (nc)
> +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
> +	else
> +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
> +
> +	rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
> +	if (rc)
> +		goto err;
> +
> +	genlmsg_end(skb, hdr);
> +	return genlmsg_unicast(net, skb, nr->snd_portid);
> +
> +err:
> +	kfree_skb(skb);
> +	return rc;
> +}
> +
> +int ncsi_send_netlink_timeout(struct ncsi_request *nr,
> +			      struct ncsi_package *np,
> +			      struct ncsi_channel *nc)
> +{
> +	struct sk_buff *skb;
> +	struct net *net;
> +	void *hdr;
> +
> +	netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);

Or this one.

> +
> +	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
> +			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
> +	if (!hdr) {
> +		kfree_skb(skb);
> +		return -EMSGSIZE;
> +	}
> +
> +	net = dev_net(nr->cmd->dev);
> +
> +	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
> +
> +	if (np)
> +		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
> +	else
> +		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
> +			    NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
> +						 nr->cmd->data)->channel)));
> +
> +	if (nc)
> +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
> +	else
> +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
> +
> +	genlmsg_end(skb, hdr);
> +	return genlmsg_unicast(net, skb, nr->snd_portid);
> +}
> +
> +int ncsi_send_netlink_err(struct net_device *dev,
> +			  u32 snd_seq,
> +			  u32 snd_portid,
> +			  struct nlmsghdr *nlhdr,
> +			  int err)
> +{
> +	struct sk_buff *skb;
> +	struct nlmsghdr *nlh;
> +	struct nlmsgerr *nle;
> +	struct net *net;
> +
> +	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	net = dev_net(dev);
> +
> +	nlh = nlmsg_put(skb, snd_portid, snd_seq,
> +			NLMSG_ERROR, sizeof(*nle), 0);
> +	nle = (struct nlmsgerr *)nlmsg_data(nlh);
> +	nle->error = err;
> +	memcpy(&nle->msg, nlhdr, sizeof(*nlh));
> +
> +	nlmsg_end(skb, nlh);
> +
> +	return nlmsg_unicast(net->genl_sock, skb, snd_portid);
> +}
> +
>  static const struct genl_ops ncsi_ops[] = {
>  	{
>  		.cmd = NCSI_CMD_PKG_INFO,
> @@ -386,6 +584,12 @@ static const struct genl_ops ncsi_ops[] = {
>  		.doit = ncsi_clear_interface_nl,
>  		.flags = GENL_ADMIN_PERM,
>  	},
> +	{
> +		.cmd = NCSI_CMD_SEND_CMD,
> +		.policy = ncsi_genl_policy,
> +		.doit = ncsi_send_cmd_nl,
> +		.flags = GENL_ADMIN_PERM,
> +	},
>  };
>  
>  static struct genl_family ncsi_genl_family __ro_after_init = {
> diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
> index 91a5c25..c4a4688 100644
> --- a/net/ncsi/ncsi-netlink.h
> +++ b/net/ncsi/ncsi-netlink.h
> @@ -14,6 +14,18 @@
>  
>  #include "internal.h"
>  
> +int ncsi_send_netlink_rsp(struct ncsi_request *nr,
> +			  struct ncsi_package *np,
> +			  struct ncsi_channel *nc);
> +int ncsi_send_netlink_timeout(struct ncsi_request *nr,
> +			      struct ncsi_package *np,
> +			      struct ncsi_channel *nc);
> +int ncsi_send_netlink_err(struct net_device *dev,
> +			  u32 snd_seq,
> +			  u32 snd_portid,
> +			  struct nlmsghdr *nlhdr,
> +			  int err);
> +
>  int ncsi_init_netlink(struct net_device *dev);
>  int ncsi_unregister_netlink(struct net_device *dev);
>  
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index d66b347..dd931d2 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -16,9 +16,11 @@
>  #include <net/ncsi.h>
>  #include <net/net_namespace.h>
>  #include <net/sock.h>
> +#include <net/genetlink.h>
>  
>  #include "internal.h"
>  #include "ncsi-pkt.h"
> +#include "ncsi-netlink.h"
>  
>  static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
>  				 unsigned short payload)
> @@ -32,15 +34,25 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
>  	 * before calling this function.
>  	 */
>  	h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
> -	if (h->common.revision != NCSI_PKT_REVISION)
> +
> +	if (h->common.revision != NCSI_PKT_REVISION) {
> +		netdev_dbg(nr->ndp->ndev.dev,
> +			   "NCSI: unsupported header revision\n");
>  		return -EINVAL;
> -	if (ntohs(h->common.length) != payload)
> +	}
> +	if (ntohs(h->common.length) != payload) {
> +		netdev_dbg(nr->ndp->ndev.dev,
> +			   "NCSI: payload length mismatched\n");
>  		return -EINVAL;
> +	}
>  
>  	/* Check on code and reason */
>  	if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
> -	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
> -		return -EINVAL;
> +	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
> +		netdev_dbg(nr->ndp->ndev.dev,
> +			   "NCSI: non zero response/reason code\n");
> +		return -EPERM;
> +	}
>  
>  	/* Validate checksum, which might be zeroes if the
>  	 * sender doesn't support checksum according to NCSI
> @@ -52,8 +64,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
>  
>  	checksum = ncsi_calculate_checksum((unsigned char *)h,
>  					   sizeof(*h) + payload - 4);
> -	if (*pchecksum != htonl(checksum))
> +
> +	if (*pchecksum != htonl(checksum)) {
> +		netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
>  		return -EINVAL;
> +	}
>  
>  	return 0;
>  }
> @@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
>  	return 0;
>  }
>  
> +static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
> +{
> +	struct ncsi_rsp_pkt *rsp;
> +	struct ncsi_dev_priv *ndp = nr->ndp;
> +	struct ncsi_package *np;
> +	struct ncsi_channel *nc;
> +	int ret;
> +
> +	/* Find the package */
> +	rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
> +	ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
> +				      &np, &nc);
> +	if (!np)
> +		return -ENODEV;
> +
> +	ret = ncsi_send_netlink_rsp(nr, np, nc);
> +
> +	return ret;
> +}
> +
>  static struct ncsi_rsp_handler {
>  	unsigned char	type;
>  	int             payload;
> @@ -1043,6 +1078,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
>  		netdev_warn(ndp->ndev.dev,
>  			    "NCSI: 'bad' packet ignored for type 0x%x\n",
>  			    hdr->type);
> +
> +		if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> +			if (ret == -EPERM)
> +				goto out_netlink;
> +			else
> +				ncsi_send_netlink_err(ndp->ndev.dev,
> +						      nr->snd_seq,
> +						      nr->snd_portid,
> +						      &nr->nlhdr,
> +						      ret);
> +		}
>  		goto out;
>  	}
>  
> @@ -1052,6 +1098,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
>  		netdev_err(ndp->ndev.dev,
>  			   "NCSI: Handler for packet type 0x%x returned %d\n",
>  			   hdr->type, ret);
> +
> +out_netlink:
> +	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> +		ret = ncsi_rsp_handler_netlink(nr);
> +		if (ret) {
> +			netdev_err(ndp->ndev.dev,
> +				   "NCSI: Netlink handler for packet type 0x%x returned %d\n",
> +				   hdr->type, ret);
> +		}
> +	}
> +
>  out:
>  	ncsi_free_request(nr);
>  	return ret;
> -- 
> 2.9.3
> 



^ permalink raw reply

* [PATCH v2 1/2] net/ncsi: Add NCSI Broadcom OEM command
From: Samuel Mendoza-Jonas @ 2018-10-10  4:39 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <20181009174806.404791-1-vijaykhemka@fb.com>

On Tue, 2018-10-09 at 10:48 -0700, Vijay Khemka wrote:
> This patch adds OEM Broadcom commands and response handling. It also
> defines OEM Get MAC Address handler to get and configure the device.
> 
> ncsi_oem_gma_handler_bcm: This handler send NCSI broadcom command for
> getting mac address.
> ncsi_rsp_handler_oem_bcm: This handles response received for all
> broadcom OEM commands.
> ncsi_rsp_handler_oem_bcm_gma: This handles get mac address response and
> set it to device.
> 
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
>  net/ncsi/Kconfig       |  6 ++++
>  net/ncsi/internal.h    |  8 +++++
>  net/ncsi/ncsi-manage.c | 70 +++++++++++++++++++++++++++++++++++++++++-
>  net/ncsi/ncsi-pkt.h    |  8 +++++
>  net/ncsi/ncsi-rsp.c    | 40 +++++++++++++++++++++++-
>  5 files changed, 130 insertions(+), 2 deletions(-)
> 

<snip>

>  static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
>  {
>  	struct ncsi_dev *nd = &ndp->ndev;
> @@ -643,9 +676,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
>  	struct ncsi_channel *nc = ndp->active_channel;
>  	struct ncsi_channel *hot_nc = NULL;
>  	struct ncsi_cmd_arg nca;
> +	struct ncsi_oem_gma_handler *nch = NULL;
>  	unsigned char index;
>  	unsigned long flags;
> -	int ret;
> +	int ret, i;
>  

I just noticed that if CONFIG_NCSI_OEM_CMD_GET_MAC is not set then this
generates unused variable warnings for i and nch. Otherwise all looking good!

Regards,
Sam



^ permalink raw reply

* [PATCH v2] clocksource/drivers/fttmr010: fix invalid interrupt register access
From: Tao Ren @ 2018-10-10  5:50 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CACRpkdbbUO9_VOtnZL+Xf_0nTTgSruaaPOLnvZ4M+NQfPLzvWQ@mail.gmail.com>

On 10/7/18, 2:03 PM, "Linus Walleij" <linus.walleij@linaro.org> wrote:

>> TIMER_INTR_MASK register (Base Address of Timer + 0x38) is not designed
>> for masking interrupts on ast2500 chips, and it's not even listed in
>> ast2400 datasheet, so it's not safe to access TIMER_INTR_MASK on aspeed
>> chips.
>>
>> Similarly, TIMER_INTR_STATE register (Base Address of Timer + 0x34) is
>> not interrupt status register on ast2400 and ast2500 chips. Although
>> there is no side effect to reset the register in fttmr010_common_init(),
>> it's just misleading to do so.
>>
>> Besides, "count_down" is renamed to "is_aspeed" in "fttmr010" structure,
>> and more comments are added so the code is more readble.
>>
>> Signed-off-by: Tao Ren <taoren@fb.com>
>> ---
>> Changes in v2:
>>   - "count_down" is renamed to "is_aspeed" in "fttmr010" structure.
>>   - more comments are added to make the code more readable.
>>    
>    Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for the review, Linus.


- Tao Ren


^ permalink raw reply

* [PATCH i2c-next v7 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
From: Jae Hyun Yoo @ 2018-10-10 15:33 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAFd5g45JkK4BnGR7yOA3eCazTZ86FyUaNdi81Ew6VBbKxS3EZA@mail.gmail.com>

On 10/9/2018 5:08 PM, Brendan Higgins wrote:
> 
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
> 

Thanks for the review, Brendan!

^ permalink raw reply

* [PATCH i2c-next v7 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
From: Jae Hyun Yoo @ 2018-10-10 15:34 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAFd5g44vi55oMkif1K+hx=O_J0_jqeQunRNiDARF0=qJ=54jiw@mail.gmail.com>

On 10/9/2018 5:00 PM, Brendan Higgins wrote:
> 
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
> 

Thank you!

^ permalink raw reply

* [PATCH i2c-next v7 3/5] dt-bindings: i2c: aspeed: Add 'bus-timeout-ms' property as an optional property
From: Jae Hyun Yoo @ 2018-10-10 15:35 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <CAFd5g44WcAE5oJCmRQKsRmbrfYPx55kdaT6xW1hzCoYAR5YQNg@mail.gmail.com>

On 10/9/2018 5:00 PM, Brendan Higgins wrote:
> 
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
> 
> Thanks for putting all the work in to do it this way!
> 

Thanks a lot for your review, Brendan!

^ permalink raw reply

* [PATCH net-next v3] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-10 15:51 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <4069c511575c0a1d0245697692d758217ce115b4.camel@mendozajonas.com>

Hi Samual,

I will address the comment and send out the v4.

Thanks,
Justin


> On Mon, 2018-10-08 at 23:13 +0000, Justin.Lee1 at 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 <sam@mendozajonas.com>
> 
> > 
> > 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 <justin.lee1@dell.com> 
> > 
> > 
> > ---
> >  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 char        id;      /* 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_list    timer;   /* 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 <net/ncsi.h>
> >  #include <net/net_namespace.h>
> >  #include <net/sock.h>
> > +#include <net/genetlink.h>
> >  
> >  #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 a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> > index 0912847..76a4bcb 100644
> > --- a/net/ncsi/ncsi-manage.c
> > +++ b/net/ncsi/ncsi-manage.c
> > @@ -19,6 +19,7 @@
> >  #include <net/addrconf.h>
> >  #include <net/ipv6.h>
> >  #include <net/if_inet6.h>
> > +#include <net/genetlink.h>
> >  
> >  #include "internal.h"
> >  #include "ncsi-pkt.h"
> > @@ -406,6 +407,9 @@ static void ncsi_request_timeout(struct timer_list *t)
> >  {
> >  	struct ncsi_request *nr = from_timer(nr, t, timer);
> >  	struct ncsi_dev_priv *ndp = nr->ndp;
> > +	struct ncsi_package *np;
> > +	struct ncsi_channel *nc;
> > +	struct ncsi_cmd_pkt *cmd;
> >  	unsigned long flags;
> >  
> >  	/* If the request already had associated response,
> > @@ -419,6 +423,18 @@ static void ncsi_request_timeout(struct timer_list *t)
> >  	}
> >  	spin_unlock_irqrestore(&ndp->lock, flags);
> >  
> > +	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> > +		if (nr->cmd) {
> > +			/* Find the package */
> > +			cmd = (struct ncsi_cmd_pkt *)
> > +			      skb_network_header(nr->cmd);
> > +			ncsi_find_package_and_channel(ndp,
> > +						      cmd->cmd.common.channel,
> > +						      &np, &nc);
> > +			ncsi_send_netlink_timeout(nr, np, nc);
> > +		}
> > +	}
> > +
> >  	/* Release the request */
> >  	ncsi_free_request(nr);
> >  }
> > diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
> > index 45f33d6..3941bf6 100644
> > --- a/net/ncsi/ncsi-netlink.c
> > +++ b/net/ncsi/ncsi-netlink.c
> > @@ -20,6 +20,7 @@
> >  #include <uapi/linux/ncsi.h>
> >  
> >  #include "internal.h"
> > +#include "ncsi-pkt.h"
> >  #include "ncsi-netlink.h"
> >  
> >  static struct genl_family ncsi_genl_family;
> > @@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
> >  	[NCSI_ATTR_PACKAGE_LIST] =	{ .type = NLA_NESTED },
> >  	[NCSI_ATTR_PACKAGE_ID] =	{ .type = NLA_U32 },
> >  	[NCSI_ATTR_CHANNEL_ID] =	{ .type = NLA_U32 },
> > +	[NCSI_ATTR_DATA] =		{ .type = NLA_BINARY, .len = 2048 },
> >  };
> >  
> >  static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
> > @@ -366,6 +368,202 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
> >  	return 0;
> >  }
> >  
> > +static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
> > +{
> > +	struct ncsi_dev_priv *ndp;
> > +
> > +	struct ncsi_cmd_arg nca;
> > +	struct ncsi_pkt_hdr *hdr;
> > +
> > +	u32 package_id, channel_id;
> > +	unsigned char *data;
> > +	int len, ret;
> > +
> > +	if (!info || !info->attrs) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	if (!info->attrs[NCSI_ATTR_IFINDEX]) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
> > +		ret = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
> > +			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
> > +	if (!ndp) {
> > +		ret = -ENODEV;
> > +		goto out;
> > +	}
> > +
> > +	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
> > +	channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
> > +
> > +	if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
> > +		ret = -ERANGE;
> > +		goto out_netlink;
> > +	}
> > +
> > +	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
> > +	if (len < sizeof(struct ncsi_pkt_hdr)) {
> > +		netdev_info(ndp->ndev.dev, "NCSI: no OEM command to send %u\n",
> > +			    package_id);
> 
> Technically we can send any command via this interface so saying "no OEM
> command" may be a little confusing as a message.
> 
> > +		ret = -EINVAL;
> > +		goto out_netlink;
> > +	} else {
> > +		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
> > +	}
> > +
> > +	hdr = (struct ncsi_pkt_hdr *)data;
> > +
> > +	nca.ndp = ndp;
> > +	nca.package = (unsigned char)package_id;
> > +	nca.channel = (unsigned char)channel_id;
> > +	nca.type = hdr->type;
> > +	nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
> > +	nca.info = info;
> > +	nca.payload = ntohs(hdr->length);
> > +	nca.data = data + sizeof(*hdr);
> > +
> > +	ret = ncsi_xmit_cmd(&nca);
> > +out_netlink:
> > +	if (ret != 0) {
> > +		netdev_err(ndp->ndev.dev,
> > +			   "NCSI: Error %d sending OEM command\n",
> > +			   ret);
> > +		ncsi_send_netlink_err(ndp->ndev.dev,
> > +				      info->snd_seq,
> > +				      info->snd_portid,
> > +				      info->nlhdr,
> > +				      ret);
> > +	}
> > +out:
> > +	return ret;
> > +}
> > +
> > +int ncsi_send_netlink_rsp(struct ncsi_request *nr,
> > +			  struct ncsi_package *np,
> > +			  struct ncsi_channel *nc)
> > +{
> > +	struct sk_buff *skb;
> > +	struct net *net;
> > +	void *hdr;
> > +	int rc;
> > +
> > +	netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
> 
> We probably don't need this message.
> 
> > +
> > +	net = dev_net(nr->rsp->dev);
> > +
> > +	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > +	if (!skb)
> > +		return -ENOMEM;
> > +
> > +	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
> > +			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
> > +	if (!hdr) {
> > +		kfree_skb(skb);
> > +		return -EMSGSIZE;
> > +	}
> > +
> > +	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
> > +	if (np)
> > +		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
> > +	if (nc)
> > +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
> > +	else
> > +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
> > +
> > +	rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
> > +	if (rc)
> > +		goto err;
> > +
> > +	genlmsg_end(skb, hdr);
> > +	return genlmsg_unicast(net, skb, nr->snd_portid);
> > +
> > +err:
> > +	kfree_skb(skb);
> > +	return rc;
> > +}
> > +
> > +int ncsi_send_netlink_timeout(struct ncsi_request *nr,
> > +			      struct ncsi_package *np,
> > +			      struct ncsi_channel *nc)
> > +{
> > +	struct sk_buff *skb;
> > +	struct net *net;
> > +	void *hdr;
> > +
> > +	netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
> 
> Or this one.
> 
> > +
> > +	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > +	if (!skb)
> > +		return -ENOMEM;
> > +
> > +	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
> > +			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
> > +	if (!hdr) {
> > +		kfree_skb(skb);
> > +		return -EMSGSIZE;
> > +	}
> > +
> > +	net = dev_net(nr->cmd->dev);
> > +
> > +	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
> > +
> > +	if (np)
> > +		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
> > +	else
> > +		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
> > +			    NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
> > +						 nr->cmd->data)->channel)));
> > +
> > +	if (nc)
> > +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
> > +	else
> > +		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
> > +
> > +	genlmsg_end(skb, hdr);
> > +	return genlmsg_unicast(net, skb, nr->snd_portid);
> > +}
> > +
> > +int ncsi_send_netlink_err(struct net_device *dev,
> > +			  u32 snd_seq,
> > +			  u32 snd_portid,
> > +			  struct nlmsghdr *nlhdr,
> > +			  int err)
> > +{
> > +	struct sk_buff *skb;
> > +	struct nlmsghdr *nlh;
> > +	struct nlmsgerr *nle;
> > +	struct net *net;
> > +
> > +	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > +	if (!skb)
> > +		return -ENOMEM;
> > +
> > +	net = dev_net(dev);
> > +
> > +	nlh = nlmsg_put(skb, snd_portid, snd_seq,
> > +			NLMSG_ERROR, sizeof(*nle), 0);
> > +	nle = (struct nlmsgerr *)nlmsg_data(nlh);
> > +	nle->error = err;
> > +	memcpy(&nle->msg, nlhdr, sizeof(*nlh));
> > +
> > +	nlmsg_end(skb, nlh);
> > +
> > +	return nlmsg_unicast(net->genl_sock, skb, snd_portid);
> > +}
> > +
> >  static const struct genl_ops ncsi_ops[] = {
> >  	{
> >  		.cmd = NCSI_CMD_PKG_INFO,
> > @@ -386,6 +584,12 @@ static const struct genl_ops ncsi_ops[] = {
> >  		.doit = ncsi_clear_interface_nl,
> >  		.flags = GENL_ADMIN_PERM,
> >  	},
> > +	{
> > +		.cmd = NCSI_CMD_SEND_CMD,
> > +		.policy = ncsi_genl_policy,
> > +		.doit = ncsi_send_cmd_nl,
> > +		.flags = GENL_ADMIN_PERM,
> > +	},
> >  };
> >  
> >  static struct genl_family ncsi_genl_family __ro_after_init = {
> > diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
> > index 91a5c25..c4a4688 100644
> > --- a/net/ncsi/ncsi-netlink.h
> > +++ b/net/ncsi/ncsi-netlink.h
> > @@ -14,6 +14,18 @@
> >  
> >  #include "internal.h"
> >  
> > +int ncsi_send_netlink_rsp(struct ncsi_request *nr,
> > +			  struct ncsi_package *np,
> > +			  struct ncsi_channel *nc);
> > +int ncsi_send_netlink_timeout(struct ncsi_request *nr,
> > +			      struct ncsi_package *np,
> > +			      struct ncsi_channel *nc);
> > +int ncsi_send_netlink_err(struct net_device *dev,
> > +			  u32 snd_seq,
> > +			  u32 snd_portid,
> > +			  struct nlmsghdr *nlhdr,
> > +			  int err);
> > +
> >  int ncsi_init_netlink(struct net_device *dev);
> >  int ncsi_unregister_netlink(struct net_device *dev);
> >  
> > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> > index d66b347..dd931d2 100644
> > --- a/net/ncsi/ncsi-rsp.c
> > +++ b/net/ncsi/ncsi-rsp.c
> > @@ -16,9 +16,11 @@
> >  #include <net/ncsi.h>
> >  #include <net/net_namespace.h>
> >  #include <net/sock.h>
> > +#include <net/genetlink.h>
> >  
> >  #include "internal.h"
> >  #include "ncsi-pkt.h"
> > +#include "ncsi-netlink.h"
> >  
> >  static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
> >  				 unsigned short payload)
> > @@ -32,15 +34,25 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
> >  	 * before calling this function.
> >  	 */
> >  	h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
> > -	if (h->common.revision != NCSI_PKT_REVISION)
> > +
> > +	if (h->common.revision != NCSI_PKT_REVISION) {
> > +		netdev_dbg(nr->ndp->ndev.dev,
> > +			   "NCSI: unsupported header revision\n");
> >  		return -EINVAL;
> > -	if (ntohs(h->common.length) != payload)
> > +	}
> > +	if (ntohs(h->common.length) != payload) {
> > +		netdev_dbg(nr->ndp->ndev.dev,
> > +			   "NCSI: payload length mismatched\n");
> >  		return -EINVAL;
> > +	}
> >  
> >  	/* Check on code and reason */
> >  	if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
> > -	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
> > -		return -EINVAL;
> > +	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
> > +		netdev_dbg(nr->ndp->ndev.dev,
> > +			   "NCSI: non zero response/reason code\n");
> > +		return -EPERM;
> > +	}
> >  
> >  	/* Validate checksum, which might be zeroes if the
> >  	 * sender doesn't support checksum according to NCSI
> > @@ -52,8 +64,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
> >  
> >  	checksum = ncsi_calculate_checksum((unsigned char *)h,
> >  					   sizeof(*h) + payload - 4);
> > -	if (*pchecksum != htonl(checksum))
> > +
> > +	if (*pchecksum != htonl(checksum)) {
> > +		netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
> >  		return -EINVAL;
> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
> >  	return 0;
> >  }
> >  
> > +static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
> > +{
> > +	struct ncsi_rsp_pkt *rsp;
> > +	struct ncsi_dev_priv *ndp = nr->ndp;
> > +	struct ncsi_package *np;
> > +	struct ncsi_channel *nc;
> > +	int ret;
> > +
> > +	/* Find the package */
> > +	rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
> > +	ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
> > +				      &np, &nc);
> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	ret = ncsi_send_netlink_rsp(nr, np, nc);
> > +
> > +	return ret;
> > +}
> > +
> >  static struct ncsi_rsp_handler {
> >  	unsigned char	type;
> >  	int             payload;
> > @@ -1043,6 +1078,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
> >  		netdev_warn(ndp->ndev.dev,
> >  			    "NCSI: 'bad' packet ignored for type 0x%x\n",
> >  			    hdr->type);
> > +
> > +		if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> > +			if (ret == -EPERM)
> > +				goto out_netlink;
> > +			else
> > +				ncsi_send_netlink_err(ndp->ndev.dev,
> > +						      nr->snd_seq,
> > +						      nr->snd_portid,
> > +						      &nr->nlhdr,
> > +						      ret);
> > +		}
> >  		goto out;
> >  	}
> >  
> > @@ -1052,6 +1098,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
> >  		netdev_err(ndp->ndev.dev,
> >  			   "NCSI: Handler for packet type 0x%x returned %d\n",
> >  			   hdr->type, ret);
> > +
> > +out_netlink:
> > +	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> > +		ret = ncsi_rsp_handler_netlink(nr);
> > +		if (ret) {
> > +			netdev_err(ndp->ndev.dev,
> > +				   "NCSI: Netlink handler for packet type 0x%x returned %d\n",
> > +				   hdr->type, ret);
> > +		}
> > +	}
> > +
> >  out:
> >  	ncsi_free_request(nr);
> >  	return ret;
> > -- 
> > 2.9.3
> > 



^ permalink raw reply

* [PATCH net-next v4] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-10 16:00 UTC (permalink / raw)
  To: linux-aspeed

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


Signed-off-by: Justin Lee <justin.lee1@dell.com> 


---
V4: Update comments and remove some debug message.
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.

include/uapi/linux/ncsi.h |   6 ++
 net/ncsi/internal.h       |  10 ++-
 net/ncsi/ncsi-cmd.c       |   8 ++
 net/ncsi/ncsi-manage.c    |  16 ++++
 net/ncsi/ncsi-netlink.c   | 200 ++++++++++++++++++++++++++++++++++++++++++++++
 net/ncsi/ncsi-netlink.h   |  12 +++
 net/ncsi/ncsi-rsp.c       |  67 ++++++++++++++--
 7 files changed, 313 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
index 4c292ec..0a26a55 100644
--- a/include/uapi/linux/ncsi.h
+++ b/include/uapi/linux/ncsi.h
@@ -23,6 +23,9 @@
  *	optionally the preferred NCSI_ATTR_CHANNEL_ID.
  * @NCSI_CMD_CLEAR_INTERFACE: clear any preferred package/channel combination.
  *	Requires NCSI_ATTR_IFINDEX.
+ * @NCSI_CMD_SEND_CMD: send NC-SI command to network card.
+ *	Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID
+ *	and NCSI_ATTR_CHANNEL_ID.
  * @NCSI_CMD_MAX: highest command number
  */
 enum ncsi_nl_commands {
@@ -30,6 +33,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 +47,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 +56,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 char        id;      /* 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_list    timer;   /* 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 <net/ncsi.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
+#include <net/genetlink.h>
 
 #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 a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0912847..76a4bcb 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -19,6 +19,7 @@
 #include <net/addrconf.h>
 #include <net/ipv6.h>
 #include <net/if_inet6.h>
+#include <net/genetlink.h>
 
 #include "internal.h"
 #include "ncsi-pkt.h"
@@ -406,6 +407,9 @@ static void ncsi_request_timeout(struct timer_list *t)
 {
 	struct ncsi_request *nr = from_timer(nr, t, timer);
 	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct ncsi_package *np;
+	struct ncsi_channel *nc;
+	struct ncsi_cmd_pkt *cmd;
 	unsigned long flags;
 
 	/* If the request already had associated response,
@@ -419,6 +423,18 @@ static void ncsi_request_timeout(struct timer_list *t)
 	}
 	spin_unlock_irqrestore(&ndp->lock, flags);
 
+	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+		if (nr->cmd) {
+			/* Find the package */
+			cmd = (struct ncsi_cmd_pkt *)
+			      skb_network_header(nr->cmd);
+			ncsi_find_package_and_channel(ndp,
+						      cmd->cmd.common.channel,
+						      &np, &nc);
+			ncsi_send_netlink_timeout(nr, np, nc);
+		}
+	}
+
 	/* Release the request */
 	ncsi_free_request(nr);
 }
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 45f33d6..62e191f 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -20,6 +20,7 @@
 #include <uapi/linux/ncsi.h>
 
 #include "internal.h"
+#include "ncsi-pkt.h"
 #include "ncsi-netlink.h"
 
 static struct genl_family ncsi_genl_family;
@@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
 	[NCSI_ATTR_PACKAGE_LIST] =	{ .type = NLA_NESTED },
 	[NCSI_ATTR_PACKAGE_ID] =	{ .type = NLA_U32 },
 	[NCSI_ATTR_CHANNEL_ID] =	{ .type = NLA_U32 },
+	[NCSI_ATTR_DATA] =		{ .type = NLA_BINARY, .len = 2048 },
 };
 
 static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
@@ -366,6 +368,198 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
 	return 0;
 }
 
+static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	struct ncsi_dev_priv *ndp;
+
+	struct ncsi_cmd_arg nca;
+	struct ncsi_pkt_hdr *hdr;
+
+	u32 package_id, channel_id;
+	unsigned char *data;
+	int len, ret;
+
+	if (!info || !info->attrs) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_IFINDEX]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
+			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+	if (!ndp) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
+	channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
+
+	if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
+		ret = -ERANGE;
+		goto out_netlink;
+	}
+
+	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
+	if (len < sizeof(struct ncsi_pkt_hdr)) {
+		netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
+			    package_id);
+		ret = -EINVAL;
+		goto out_netlink;
+	} else {
+		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
+	}
+
+	hdr = (struct ncsi_pkt_hdr *)data;
+
+	nca.ndp = ndp;
+	nca.package = (unsigned char)package_id;
+	nca.channel = (unsigned char)channel_id;
+	nca.type = hdr->type;
+	nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
+	nca.info = info;
+	nca.payload = ntohs(hdr->length);
+	nca.data = data + sizeof(*hdr);
+
+	ret = ncsi_xmit_cmd(&nca);
+out_netlink:
+	if (ret != 0) {
+		netdev_err(ndp->ndev.dev,
+			   "NCSI: Error %d sending OEM command\n",
+			   ret);
+		ncsi_send_netlink_err(ndp->ndev.dev,
+				      info->snd_seq,
+				      info->snd_portid,
+				      info->nlhdr,
+				      ret);
+	}
+out:
+	return ret;
+}
+
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+			  struct ncsi_package *np,
+			  struct ncsi_channel *nc)
+{
+	struct sk_buff *skb;
+	struct net *net;
+	void *hdr;
+	int rc;
+
+	net = dev_net(nr->rsp->dev);
+
+	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+	if (!hdr) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
+	if (np)
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+	if (nc)
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+	rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
+	if (rc)
+		goto err;
+
+	genlmsg_end(skb, hdr);
+	return genlmsg_unicast(net, skb, nr->snd_portid);
+
+err:
+	kfree_skb(skb);
+	return rc;
+}
+
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+			      struct ncsi_package *np,
+			      struct ncsi_channel *nc)
+{
+	struct sk_buff *skb;
+	struct net *net;
+	void *hdr;
+
+	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+	if (!hdr) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	net = dev_net(nr->cmd->dev);
+
+	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
+
+	if (np)
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
+			    NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
+						 nr->cmd->data)->channel)));
+
+	if (nc)
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+	genlmsg_end(skb, hdr);
+	return genlmsg_unicast(net, skb, nr->snd_portid);
+}
+
+int ncsi_send_netlink_err(struct net_device *dev,
+			  u32 snd_seq,
+			  u32 snd_portid,
+			  struct nlmsghdr *nlhdr,
+			  int err)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr *nlh;
+	struct nlmsgerr *nle;
+	struct net *net;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	net = dev_net(dev);
+
+	nlh = nlmsg_put(skb, snd_portid, snd_seq,
+			NLMSG_ERROR, sizeof(*nle), 0);
+	nle = (struct nlmsgerr *)nlmsg_data(nlh);
+	nle->error = err;
+	memcpy(&nle->msg, nlhdr, sizeof(*nlh));
+
+	nlmsg_end(skb, nlh);
+
+	return nlmsg_unicast(net->genl_sock, skb, snd_portid);
+}
+
 static const struct genl_ops ncsi_ops[] = {
 	{
 		.cmd = NCSI_CMD_PKG_INFO,
@@ -386,6 +580,12 @@ static const struct genl_ops ncsi_ops[] = {
 		.doit = ncsi_clear_interface_nl,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = NCSI_CMD_SEND_CMD,
+		.policy = ncsi_genl_policy,
+		.doit = ncsi_send_cmd_nl,
+		.flags = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family ncsi_genl_family __ro_after_init = {
diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
index 91a5c25..c4a4688 100644
--- a/net/ncsi/ncsi-netlink.h
+++ b/net/ncsi/ncsi-netlink.h
@@ -14,6 +14,18 @@
 
 #include "internal.h"
 
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+			  struct ncsi_package *np,
+			  struct ncsi_channel *nc);
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+			      struct ncsi_package *np,
+			      struct ncsi_channel *nc);
+int ncsi_send_netlink_err(struct net_device *dev,
+			  u32 snd_seq,
+			  u32 snd_portid,
+			  struct nlmsghdr *nlhdr,
+			  int err);
+
 int ncsi_init_netlink(struct net_device *dev);
 int ncsi_unregister_netlink(struct net_device *dev);
 
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b347..dd931d2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -16,9 +16,11 @@
 #include <net/ncsi.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
+#include <net/genetlink.h>
 
 #include "internal.h"
 #include "ncsi-pkt.h"
+#include "ncsi-netlink.h"
 
 static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 				 unsigned short payload)
@@ -32,15 +34,25 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 	 * before calling this function.
 	 */
 	h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
-	if (h->common.revision != NCSI_PKT_REVISION)
+
+	if (h->common.revision != NCSI_PKT_REVISION) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: unsupported header revision\n");
 		return -EINVAL;
-	if (ntohs(h->common.length) != payload)
+	}
+	if (ntohs(h->common.length) != payload) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: payload length mismatched\n");
 		return -EINVAL;
+	}
 
 	/* Check on code and reason */
 	if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
-	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
-		return -EINVAL;
+	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: non zero response/reason code\n");
+		return -EPERM;
+	}
 
 	/* Validate checksum, which might be zeroes if the
 	 * sender doesn't support checksum according to NCSI
@@ -52,8 +64,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 
 	checksum = ncsi_calculate_checksum((unsigned char *)h,
 					   sizeof(*h) + payload - 4);
-	if (*pchecksum != htonl(checksum))
+
+	if (*pchecksum != htonl(checksum)) {
+		netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
 	return 0;
 }
 
+static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_pkt *rsp;
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct ncsi_package *np;
+	struct ncsi_channel *nc;
+	int ret;
+
+	/* Find the package */
+	rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
+	ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
+				      &np, &nc);
+	if (!np)
+		return -ENODEV;
+
+	ret = ncsi_send_netlink_rsp(nr, np, nc);
+
+	return ret;
+}
+
 static struct ncsi_rsp_handler {
 	unsigned char	type;
 	int             payload;
@@ -1043,6 +1078,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 		netdev_warn(ndp->ndev.dev,
 			    "NCSI: 'bad' packet ignored for type 0x%x\n",
 			    hdr->type);
+
+		if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+			if (ret == -EPERM)
+				goto out_netlink;
+			else
+				ncsi_send_netlink_err(ndp->ndev.dev,
+						      nr->snd_seq,
+						      nr->snd_portid,
+						      &nr->nlhdr,
+						      ret);
+		}
 		goto out;
 	}
 
@@ -1052,6 +1098,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 		netdev_err(ndp->ndev.dev,
 			   "NCSI: Handler for packet type 0x%x returned %d\n",
 			   hdr->type, ret);
+
+out_netlink:
+	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+		ret = ncsi_rsp_handler_netlink(nr);
+		if (ret) {
+			netdev_err(ndp->ndev.dev,
+				   "NCSI: Netlink handler for packet type 0x%x returned %d\n",
+				   hdr->type, ret);
+		}
+	}
+
 out:
 	ncsi_free_request(nr);
 	return ret;
-- 
2.9.3



^ permalink raw reply related

* [PATCH net-next v4] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Vijay Khemka @ 2018-10-10 17:19 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <7c7f95baba3a467e89aabf726a5230f7@AUSX13MPS302.AMER.DELL.COM>

Hi Justin,
Please see minor comments below.

Regards
-Vijay

?On 10/10/18, 9:01 AM, "Justin.Lee1 at Dell.com" <Justin.Lee1@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
    
    
    Signed-off-by: Justin Lee <justin.lee1@dell.com> 
    
    
    ---
    V4: Update comments and remove some debug message.
    V3: Based on https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_979688_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=v9MU0Ki9pWnTXCWwjHPVgpnCR80vXkkcrIaqU7USl5g&m=38KSrF_ThuoRmouLx-xKKkkpk9EfhNFEEqXbduQqQpE&s=GRXR1sIEzNo7xDKyUWS9t1Ita6vxEX_llzMx2azueVc&e= to remove the duplicated code.
    V2: Remove non-related debug message and clean up the code.
    
    
    --- 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 char        id;      /* 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
  
  Why extra space added above?

    +#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_list    timer;   /* 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          */
     };
     
  Extra line inside structure may not be required.

     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           */
     };
     
    diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
    index 45f33d6..62e191f 100644
    --- a/net/ncsi/ncsi-netlink.c
    +++ b/net/ncsi/ncsi-netlink.c
    @@ -20,6 +20,7 @@
     #include <uapi/linux/ncsi.h>
     
     #include "internal.h"
    +#include "ncsi-pkt.h"
     #include "ncsi-netlink.h"
     
     static struct genl_family ncsi_genl_family;
    @@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
     	[NCSI_ATTR_PACKAGE_LIST] =	{ .type = NLA_NESTED },
     	[NCSI_ATTR_PACKAGE_ID] =	{ .type = NLA_U32 },
     	[NCSI_ATTR_CHANNEL_ID] =	{ .type = NLA_U32 },
    +	[NCSI_ATTR_DATA] =		{ .type = NLA_BINARY, .len = 2048 },
     };
     
     static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
    @@ -366,6 +368,198 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
     	return 0;
     }
     
    +static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
    +{
    +	struct ncsi_dev_priv *ndp;
    +
    +	struct ncsi_cmd_arg nca;
    +	struct ncsi_pkt_hdr *hdr;
    +
    +	u32 package_id, channel_id;
    +	unsigned char *data;
    +	int len, ret;
    +
    +	if (!info || !info->attrs) {
    +		ret = -EINVAL;
    +		goto out;
    +	}
    +
    +	if (!info->attrs[NCSI_ATTR_IFINDEX]) {
    +		ret = -EINVAL;
    +		goto out;
    +	}
    +
    +	if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
    +		ret = -EINVAL;
    +		goto out;
    +	}
    +
    +	if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
    +		ret = -EINVAL;
    +		goto out;
    +	}
    +
    +	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
    +			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
    +	if (!ndp) {
    +		ret = -ENODEV;
    +		goto out;
    +	}
    +
    +	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
    +	channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
    +
    +	if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
    +		ret = -ERANGE;
    +		goto out_netlink;
    +	}
    +
    +	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
    +	if (len < sizeof(struct ncsi_pkt_hdr)) {
    +		netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
    +			    package_id);
    +		ret = -EINVAL;
    +		goto out_netlink;
    +	} else {
    +		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
    +	}
    +
    +	hdr = (struct ncsi_pkt_hdr *)data;
    +
    +	nca.ndp = ndp;
    +	nca.package = (unsigned char)package_id;
    +	nca.channel = (unsigned char)channel_id;
    +	nca.type = hdr->type;
    +	nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
    +	nca.info = info;
    +	nca.payload = ntohs(hdr->length);
    +	nca.data = data + sizeof(*hdr);
    +
    +	ret = ncsi_xmit_cmd(&nca);
    +out_netlink:
    +	if (ret != 0) {
    +		netdev_err(ndp->ndev.dev,
    +			   "NCSI: Error %d sending OEM command\n",
    +			   ret);
    +		ncsi_send_netlink_err(ndp->ndev.dev,
    +				      info->snd_seq,
    +				      info->snd_portid,
    +				      info->nlhdr,
    +				      ret);
    +	}

  OEM should be removed from above message.

    +out:
    +	return ret;
    +}
    +
     
    
    


^ permalink raw reply

* [PATCH net-next v4] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-10 17:50 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <57729D0A-A210-41F4-BAF8-3BDB2FCBF1B5@fb.com>

Hi Vijay,

I will address the comment and send out the v5.

Thanks,
Justin


> Hi Justin,
> Please see minor comments below.
> 
> Regards
> -Vijay
> 
> On 10/10/18, 9:01 AM, "Justin.Lee1 at Dell.com" <Justin.Lee1@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
>     
>     
>     Signed-off-by: Justin Lee <justin.lee1@dell.com> 
>     
>     
>     ---
>     V4: Update comments and remove some debug message.
>     V3: Based on https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_979688_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=v9MU0Ki9pWnTXCWwjHPVgpnCR80vXkkcrIaqU7USl5g&m=38KSrF_ThuoRmouLx-xKKkkpk9EfhNFEEqXbduQqQpE&s=GRXR1sIEzNo7xDKyUWS9t1Ita6vxEX_llzMx2azueVc&e= to remove the duplicated code.
>     V2: Remove non-related debug message and clean up the code.
>     
>     
>     --- 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 char        id;      /* 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
>   
>   Why extra space added above?
> 
>     +#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_list    timer;   /* 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          */
>      };
>      
>   Extra line inside structure may not be required.
> 
>      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           */
>      };
>      
>     diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
>     index 45f33d6..62e191f 100644
>     --- a/net/ncsi/ncsi-netlink.c
>     +++ b/net/ncsi/ncsi-netlink.c
>     @@ -20,6 +20,7 @@
>      #include <uapi/linux/ncsi.h>
>      
>      #include "internal.h"
>     +#include "ncsi-pkt.h"
>      #include "ncsi-netlink.h"
>      
>      static struct genl_family ncsi_genl_family;
>     @@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
>      	[NCSI_ATTR_PACKAGE_LIST] =	{ .type = NLA_NESTED },
>      	[NCSI_ATTR_PACKAGE_ID] =	{ .type = NLA_U32 },
>      	[NCSI_ATTR_CHANNEL_ID] =	{ .type = NLA_U32 },
>     +	[NCSI_ATTR_DATA] =		{ .type = NLA_BINARY, .len = 2048 },
>      };
>      
>      static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
>     @@ -366,6 +368,198 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
>      	return 0;
>      }
>      
>     +static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
>     +{
>     +	struct ncsi_dev_priv *ndp;
>     +
>     +	struct ncsi_cmd_arg nca;
>     +	struct ncsi_pkt_hdr *hdr;
>     +
>     +	u32 package_id, channel_id;
>     +	unsigned char *data;
>     +	int len, ret;
>     +
>     +	if (!info || !info->attrs) {
>     +		ret = -EINVAL;
>     +		goto out;
>     +	}
>     +
>     +	if (!info->attrs[NCSI_ATTR_IFINDEX]) {
>     +		ret = -EINVAL;
>     +		goto out;
>     +	}
>     +
>     +	if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
>     +		ret = -EINVAL;
>     +		goto out;
>     +	}
>     +
>     +	if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
>     +		ret = -EINVAL;
>     +		goto out;
>     +	}
>     +
>     +	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
>     +			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
>     +	if (!ndp) {
>     +		ret = -ENODEV;
>     +		goto out;
>     +	}
>     +
>     +	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
>     +	channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
>     +
>     +	if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
>     +		ret = -ERANGE;
>     +		goto out_netlink;
>     +	}
>     +
>     +	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
>     +	if (len < sizeof(struct ncsi_pkt_hdr)) {
>     +		netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
>     +			    package_id);
>     +		ret = -EINVAL;
>     +		goto out_netlink;
>     +	} else {
>     +		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
>     +	}
>     +
>     +	hdr = (struct ncsi_pkt_hdr *)data;
>     +
>     +	nca.ndp = ndp;
>     +	nca.package = (unsigned char)package_id;
>     +	nca.channel = (unsigned char)channel_id;
>     +	nca.type = hdr->type;
>     +	nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
>     +	nca.info = info;
>     +	nca.payload = ntohs(hdr->length);
>     +	nca.data = data + sizeof(*hdr);
>     +
>     +	ret = ncsi_xmit_cmd(&nca);
>     +out_netlink:
>     +	if (ret != 0) {
>     +		netdev_err(ndp->ndev.dev,
>     +			   "NCSI: Error %d sending OEM command\n",
>     +			   ret);
>     +		ncsi_send_netlink_err(ndp->ndev.dev,
>     +				      info->snd_seq,
>     +				      info->snd_portid,
>     +				      info->nlhdr,
>     +				      ret);
>     +	}
> 
>   OEM should be removed from above message.
> 
>     +out:
>     +	return ret;
>     +}
>     +



^ permalink raw reply

* [PATCH net-next v5] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-10-10 18:11 UTC (permalink / raw)
  To: linux-aspeed

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


Signed-off-by: Justin Lee <justin.lee1@dell.com> 


---
V5: Update comments and debug message.
V4: Update comments and remove some debug message.
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.

 include/uapi/linux/ncsi.h |   6 ++
 net/ncsi/internal.h       |   7 ++
 net/ncsi/ncsi-cmd.c       |   8 ++
 net/ncsi/ncsi-manage.c    |  16 ++++
 net/ncsi/ncsi-netlink.c   | 200 ++++++++++++++++++++++++++++++++++++++++++++++
 net/ncsi/ncsi-netlink.h   |  12 +++
 net/ncsi/ncsi-rsp.c       |  67 ++++++++++++++--
 7 files changed, 311 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
index 4c292ec..0a26a55 100644
--- a/include/uapi/linux/ncsi.h
+++ b/include/uapi/linux/ncsi.h
@@ -23,6 +23,9 @@
  *	optionally the preferred NCSI_ATTR_CHANNEL_ID.
  * @NCSI_CMD_CLEAR_INTERFACE: clear any preferred package/channel combination.
  *	Requires NCSI_ATTR_IFINDEX.
+ * @NCSI_CMD_SEND_CMD: send NC-SI command to network card.
+ *	Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID
+ *	and NCSI_ATTR_CHANNEL_ID.
  * @NCSI_CMD_MAX: highest command number
  */
 enum ncsi_nl_commands {
@@ -30,6 +33,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 +47,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 +56,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..13c9b5e 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;
@@ -220,11 +222,15 @@ struct ncsi_request {
 	bool                 used;    /* Request that has been assigned  */
 	unsigned int         flags;   /* NCSI request property           */
 #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_list    timer;   /* 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 +316,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 <net/ncsi.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
+#include <net/genetlink.h>
 
 #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 a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0912847..76a4bcb 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -19,6 +19,7 @@
 #include <net/addrconf.h>
 #include <net/ipv6.h>
 #include <net/if_inet6.h>
+#include <net/genetlink.h>
 
 #include "internal.h"
 #include "ncsi-pkt.h"
@@ -406,6 +407,9 @@ static void ncsi_request_timeout(struct timer_list *t)
 {
 	struct ncsi_request *nr = from_timer(nr, t, timer);
 	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct ncsi_package *np;
+	struct ncsi_channel *nc;
+	struct ncsi_cmd_pkt *cmd;
 	unsigned long flags;
 
 	/* If the request already had associated response,
@@ -419,6 +423,18 @@ static void ncsi_request_timeout(struct timer_list *t)
 	}
 	spin_unlock_irqrestore(&ndp->lock, flags);
 
+	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+		if (nr->cmd) {
+			/* Find the package */
+			cmd = (struct ncsi_cmd_pkt *)
+			      skb_network_header(nr->cmd);
+			ncsi_find_package_and_channel(ndp,
+						      cmd->cmd.common.channel,
+						      &np, &nc);
+			ncsi_send_netlink_timeout(nr, np, nc);
+		}
+	}
+
 	/* Release the request */
 	ncsi_free_request(nr);
 }
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index 45f33d6..eaee570 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -20,6 +20,7 @@
 #include <uapi/linux/ncsi.h>
 
 #include "internal.h"
+#include "ncsi-pkt.h"
 #include "ncsi-netlink.h"
 
 static struct genl_family ncsi_genl_family;
@@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
 	[NCSI_ATTR_PACKAGE_LIST] =	{ .type = NLA_NESTED },
 	[NCSI_ATTR_PACKAGE_ID] =	{ .type = NLA_U32 },
 	[NCSI_ATTR_CHANNEL_ID] =	{ .type = NLA_U32 },
+	[NCSI_ATTR_DATA] =		{ .type = NLA_BINARY, .len = 2048 },
 };
 
 static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
@@ -366,6 +368,198 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
 	return 0;
 }
 
+static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
+{
+	struct ncsi_dev_priv *ndp;
+
+	struct ncsi_cmd_arg nca;
+	struct ncsi_pkt_hdr *hdr;
+
+	u32 package_id, channel_id;
+	unsigned char *data;
+	int len, ret;
+
+	if (!info || !info->attrs) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_IFINDEX]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
+			       nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
+	if (!ndp) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
+	channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
+
+	if (package_id >= NCSI_MAX_PACKAGE || channel_id >= NCSI_MAX_CHANNEL) {
+		ret = -ERANGE;
+		goto out_netlink;
+	}
+
+	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
+	if (len < sizeof(struct ncsi_pkt_hdr)) {
+		netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
+			    package_id);
+		ret = -EINVAL;
+		goto out_netlink;
+	} else {
+		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
+	}
+
+	hdr = (struct ncsi_pkt_hdr *)data;
+
+	nca.ndp = ndp;
+	nca.package = (unsigned char)package_id;
+	nca.channel = (unsigned char)channel_id;
+	nca.type = hdr->type;
+	nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
+	nca.info = info;
+	nca.payload = ntohs(hdr->length);
+	nca.data = data + sizeof(*hdr);
+
+	ret = ncsi_xmit_cmd(&nca);
+out_netlink:
+	if (ret != 0) {
+		netdev_err(ndp->ndev.dev,
+			   "NCSI: Error %d sending command\n",
+			   ret);
+		ncsi_send_netlink_err(ndp->ndev.dev,
+				      info->snd_seq,
+				      info->snd_portid,
+				      info->nlhdr,
+				      ret);
+	}
+out:
+	return ret;
+}
+
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+			  struct ncsi_package *np,
+			  struct ncsi_channel *nc)
+{
+	struct sk_buff *skb;
+	struct net *net;
+	void *hdr;
+	int rc;
+
+	net = dev_net(nr->rsp->dev);
+
+	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+	if (!hdr) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
+	if (np)
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+	if (nc)
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+	rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
+	if (rc)
+		goto err;
+
+	genlmsg_end(skb, hdr);
+	return genlmsg_unicast(net, skb, nr->snd_portid);
+
+err:
+	kfree_skb(skb);
+	return rc;
+}
+
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+			      struct ncsi_package *np,
+			      struct ncsi_channel *nc)
+{
+	struct sk_buff *skb;
+	struct net *net;
+	void *hdr;
+
+	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
+			  &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
+	if (!hdr) {
+		kfree_skb(skb);
+		return -EMSGSIZE;
+	}
+
+	net = dev_net(nr->cmd->dev);
+
+	nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
+
+	if (np)
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
+			    NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)
+						 nr->cmd->data)->channel)));
+
+	if (nc)
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
+	else
+		nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
+
+	genlmsg_end(skb, hdr);
+	return genlmsg_unicast(net, skb, nr->snd_portid);
+}
+
+int ncsi_send_netlink_err(struct net_device *dev,
+			  u32 snd_seq,
+			  u32 snd_portid,
+			  struct nlmsghdr *nlhdr,
+			  int err)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr *nlh;
+	struct nlmsgerr *nle;
+	struct net *net;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	net = dev_net(dev);
+
+	nlh = nlmsg_put(skb, snd_portid, snd_seq,
+			NLMSG_ERROR, sizeof(*nle), 0);
+	nle = (struct nlmsgerr *)nlmsg_data(nlh);
+	nle->error = err;
+	memcpy(&nle->msg, nlhdr, sizeof(*nlh));
+
+	nlmsg_end(skb, nlh);
+
+	return nlmsg_unicast(net->genl_sock, skb, snd_portid);
+}
+
 static const struct genl_ops ncsi_ops[] = {
 	{
 		.cmd = NCSI_CMD_PKG_INFO,
@@ -386,6 +580,12 @@ static const struct genl_ops ncsi_ops[] = {
 		.doit = ncsi_clear_interface_nl,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = NCSI_CMD_SEND_CMD,
+		.policy = ncsi_genl_policy,
+		.doit = ncsi_send_cmd_nl,
+		.flags = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family ncsi_genl_family __ro_after_init = {
diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
index 91a5c25..c4a4688 100644
--- a/net/ncsi/ncsi-netlink.h
+++ b/net/ncsi/ncsi-netlink.h
@@ -14,6 +14,18 @@
 
 #include "internal.h"
 
+int ncsi_send_netlink_rsp(struct ncsi_request *nr,
+			  struct ncsi_package *np,
+			  struct ncsi_channel *nc);
+int ncsi_send_netlink_timeout(struct ncsi_request *nr,
+			      struct ncsi_package *np,
+			      struct ncsi_channel *nc);
+int ncsi_send_netlink_err(struct net_device *dev,
+			  u32 snd_seq,
+			  u32 snd_portid,
+			  struct nlmsghdr *nlhdr,
+			  int err);
+
 int ncsi_init_netlink(struct net_device *dev);
 int ncsi_unregister_netlink(struct net_device *dev);
 
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index d66b347..dd931d2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -16,9 +16,11 @@
 #include <net/ncsi.h>
 #include <net/net_namespace.h>
 #include <net/sock.h>
+#include <net/genetlink.h>
 
 #include "internal.h"
 #include "ncsi-pkt.h"
+#include "ncsi-netlink.h"
 
 static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 				 unsigned short payload)
@@ -32,15 +34,25 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 	 * before calling this function.
 	 */
 	h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
-	if (h->common.revision != NCSI_PKT_REVISION)
+
+	if (h->common.revision != NCSI_PKT_REVISION) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: unsupported header revision\n");
 		return -EINVAL;
-	if (ntohs(h->common.length) != payload)
+	}
+	if (ntohs(h->common.length) != payload) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: payload length mismatched\n");
 		return -EINVAL;
+	}
 
 	/* Check on code and reason */
 	if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
-	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
-		return -EINVAL;
+	    ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: non zero response/reason code\n");
+		return -EPERM;
+	}
 
 	/* Validate checksum, which might be zeroes if the
 	 * sender doesn't support checksum according to NCSI
@@ -52,8 +64,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 
 	checksum = ncsi_calculate_checksum((unsigned char *)h,
 					   sizeof(*h) + payload - 4);
-	if (*pchecksum != htonl(checksum))
+
+	if (*pchecksum != htonl(checksum)) {
+		netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -941,6 +956,26 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
 	return 0;
 }
 
+static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
+{
+	struct ncsi_rsp_pkt *rsp;
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct ncsi_package *np;
+	struct ncsi_channel *nc;
+	int ret;
+
+	/* Find the package */
+	rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
+	ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
+				      &np, &nc);
+	if (!np)
+		return -ENODEV;
+
+	ret = ncsi_send_netlink_rsp(nr, np, nc);
+
+	return ret;
+}
+
 static struct ncsi_rsp_handler {
 	unsigned char	type;
 	int             payload;
@@ -1043,6 +1078,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 		netdev_warn(ndp->ndev.dev,
 			    "NCSI: 'bad' packet ignored for type 0x%x\n",
 			    hdr->type);
+
+		if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+			if (ret == -EPERM)
+				goto out_netlink;
+			else
+				ncsi_send_netlink_err(ndp->ndev.dev,
+						      nr->snd_seq,
+						      nr->snd_portid,
+						      &nr->nlhdr,
+						      ret);
+		}
 		goto out;
 	}
 
@@ -1052,6 +1098,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 		netdev_err(ndp->ndev.dev,
 			   "NCSI: Handler for packet type 0x%x returned %d\n",
 			   hdr->type, ret);
+
+out_netlink:
+	if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
+		ret = ncsi_rsp_handler_netlink(nr);
+		if (ret) {
+			netdev_err(ndp->ndev.dev,
+				   "NCSI: Netlink handler for packet type 0x%x returned %d\n",
+				   hdr->type, ret);
+		}
+	}
+
 out:
 	ncsi_free_request(nr);
 	return ret;
-- 
2.9.3



^ permalink raw reply related

* [PATCH net-next v5] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Vijay Khemka @ 2018-10-10 21:30 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <245b32d9a3dd463f9334b9704b639ec4@AUSX13MPS302.AMER.DELL.COM>



?On 10/10/18, 11:12 AM, "Justin.Lee1 at Dell.com" <Justin.Lee1@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
    
    
    Signed-off-by: Justin Lee <justin.lee1@dell.com> 

  Reviewed-by : Vijay Khemka <vijaykhemka@fb.com>
  


^ permalink raw reply

* [PATCH net-next v5] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Samuel Mendoza-Jonas @ 2018-10-11  3:55 UTC (permalink / raw)
  To: linux-aspeed
In-Reply-To: <245b32d9a3dd463f9334b9704b639ec4@AUSX13MPS302.AMER.DELL.COM>

On Wed, 2018-10-10 at 18:11 +0000, Justin.Lee1 at Dell.com wrote:
<snip>
> +
> +	len = nla_len(info->attrs[NCSI_ATTR_DATA]);
> +	if (len < sizeof(struct ncsi_pkt_hdr)) {
> +		netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
> +			    package_id);
> +		ret = -EINVAL;
> +		goto out_netlink;
> +	} else {
> +		data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
> +	}

I only just noticed this, the call to nla_len() can cause a null-dereference if
the NCSI_ATTR_DATA attribute isn't present; we need to make sure it exists
before accessing it in info->attrs.

eg:

root at ozrom2-bmc:~# ./ncsi-netlink -l 2 -p 0 -c 0 --cmd
[   81.399837] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[   81.409092] pgd = ddaa9fa6
[   81.413084] [00000000] *pgd=9702c831, *pte=00000000, *ppte=00000000
[   81.420729] Internal error: Oops: 17 [#1] ARM
[   81.426447] CPU: 0 PID: 1028 Comm: ncsi-netlink Not tainted 4.18.8-sammj-00144-gbc129f31bfa5 #12
...
[   81.874434] Kernel panic - not syncing: Fatal exception

Cheers,
Sam



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox