lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 3 Dec 2021 16:15:27 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Hayes Wang <hayeswang@...ltek.com>
Cc:     netdev@...r.kernel.org, nic_swsd@...ltek.com
Subject: Re: [RFC PATCH 4/4] r8169: add sysfs for dash

On 29.11.2021 11:13, Hayes Wang wrote:
> Add the sysfs for dash. Then the application could configure the
> firmware through them.
> 
> Signed-off-by: Hayes Wang <hayeswang@...ltek.com>
> ---
>  drivers/net/ethernet/realtek/r8169_dash.c | 131 +++++++++++++++
>  drivers/net/ethernet/realtek/r8169_dash.h |   8 +
>  drivers/net/ethernet/realtek/r8169_main.c | 188 +++++++++++++++++++++-
>  3 files changed, 326 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_dash.c b/drivers/net/ethernet/realtek/r8169_dash.c
> index acee7519e9f1..197feb2c4a23 100644
> --- a/drivers/net/ethernet/realtek/r8169_dash.c
> +++ b/drivers/net/ethernet/realtek/r8169_dash.c
> @@ -754,3 +754,134 @@ void rtl_dash_interrupt(struct rtl_dash *dash)
>  	tasklet_schedule(&dash->tl);
>  }
>  
> +int rtl_dash_to_fw(struct rtl_dash *dash, u8 *src, int size)
> +{
> +	int ret;
> +	long t;
> +
> +	if (dash->cmac_state != CMAC_STATE_RUNNING)
> +		return -ENETDOWN;
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = dash_rx_data(dash, NULL, 0);
> +	spin_unlock_bh(&dash->cmac_lock);
> +	if (ret < 0)
> +		return -EUCLEAN;
> +
> +	reinit_completion(&dash->cmac_tx);
> +	reinit_completion(&dash->fw_ack);
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = cmac_start_xmit(dash, src, size, false);
> +	spin_unlock_bh(&dash->cmac_lock);
> +
> +	if (ret < 0)
> +		goto err;
> +
> +	t = wait_for_completion_interruptible_timeout(&dash->cmac_tx,
> +						      CMAC_TIMEOUT);
> +	if (!t) {
> +		ret = -ETIMEDOUT;
> +		dev_err(&dash->pdev_cmac->dev, "CMAC tx timeout\n");
> +		goto err;
> +	} else if (t < 0) {
> +		ret = t;
> +		dev_err(&dash->pdev_cmac->dev, "CMAC tx fail %ld\n", t);
> +		goto err;
> +	}
> +
> +	t = wait_for_completion_interruptible_timeout(&dash->fw_ack,
> +						      CMAC_TIMEOUT);
> +	if (!t) {
> +		ret = -ETIMEDOUT;
> +		dev_err(&dash->pdev_cmac->dev, "FW ACK timeout\n");
> +	} else if (t < 0) {
> +		ret = t;
> +		dev_err(&dash->pdev_cmac->dev, "FW ACK fail %ld\n", t);
> +	}
> +
> +err:
> +	return ret;
> +}
> +
> +int rtl_dash_from_fw(struct rtl_dash *dash, u8 *src, int size)
> +{
> +	int ret;
> +	long t;
> +
> +	if (dash->cmac_state != CMAC_STATE_RUNNING)
> +		return -ENETDOWN;
> +
> +	reinit_completion(&dash->cmac_rx);
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = dash_rx_data(dash, src, size);
> +	spin_unlock_bh(&dash->cmac_lock);
> +
> +	if (ret)
> +		goto out;
> +
> +	t = wait_for_completion_interruptible_timeout(&dash->cmac_rx,
> +						      CMAC_TIMEOUT);
> +	if (!t) {
> +		dev_warn(&dash->pdev_cmac->dev, "CMAC data timeout\n");
> +	} else if (t < 0) {
> +		ret = t;
> +		dev_err(&dash->pdev_cmac->dev, "Wait CMAC data fail %ld\n", t);
> +		goto out;
> +	}
> +
> +	spin_lock_bh(&dash->cmac_lock);
> +	ret = dash_rx_data(dash, src, size);
> +	spin_unlock_bh(&dash->cmac_lock);
> +
> +out:
> +	return ret;
> +}
> +
> +void rtl_dash_set_ap_ready(struct rtl_dash *dash, bool enable)
> +{
> +	struct rtl8169_private *tp = dash->tp;
> +	u32 data;
> +
> +	data = r8168ep_ocp_read(tp, 0x124);
> +	if (enable)
> +		data |= BIT(1);
> +	else
> +		data &= ~BIT(1);
> +	r8168ep_ocp_write(tp, 0x1, 0x124, data);
> +}
> +
> +bool rtl_dash_get_ap_ready(struct rtl_dash *dash)
> +{
> +	struct rtl8169_private *tp = dash->tp;
> +
> +	if (r8168ep_ocp_read(tp, 0x124) & BIT(1))
> +		return true;
> +	else
> +		return false;
> +}
> +
> +ssize_t rtl_dash_info(struct rtl_dash *dash, char *buf)
> +{
> +	struct rtl8169_private *tp = dash->tp;
> +	char *dest = buf;
> +	int size;
> +	u32 data;
> +
> +	data = r8168ep_ocp_read(tp, 0x120);
> +	size = sprintf(dest, "FW_VERSION=0x%08X\n", data);
> +	if (size > 0)
> +		dest += size;
> +	else
> +		return size;
> +
> +	data = r8168ep_ocp_read(tp, 0x174);
> +	size = sprintf(dest, "FW_BUILD=0x%08X\n", data);
> +	if (size > 0)
> +		dest += size;
> +	else
> +		return size;
> +
> +	return strlen(buf) + 1;
> +}
> diff --git a/drivers/net/ethernet/realtek/r8169_dash.h b/drivers/net/ethernet/realtek/r8169_dash.h
> index 1e9a54a3df1b..b33f3adeef13 100644
> --- a/drivers/net/ethernet/realtek/r8169_dash.h
> +++ b/drivers/net/ethernet/realtek/r8169_dash.h
> @@ -16,6 +16,14 @@ void rtl_dash_up(struct rtl_dash *dash);
>  void rtl_dash_down(struct rtl_dash *dash);
>  void rtl_dash_cmac_reset_indicate(struct rtl_dash *dash);
>  void rtl_dash_interrupt(struct rtl_dash *dash);
> +void rtl_dash_set_ap_ready(struct rtl_dash *dash, bool enable);
> +
> +int rtl_dash_to_fw(struct rtl_dash *dash, u8 *src, int size);
> +int rtl_dash_from_fw(struct rtl_dash *dash, u8 *src, int size);
> +
> +bool rtl_dash_get_ap_ready(struct rtl_dash *dash);
> +
> +ssize_t rtl_dash_info(struct rtl_dash *dash, char *buf);
>  
>  struct rtl_dash *rtl_request_dash(struct rtl8169_private *tp,
>  				  struct pci_dev *pci_dev, enum mac_version ver,
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 83da05e5769e..4c8439d3ae4d 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -4654,6 +4654,184 @@ static int r8169_phy_connect(struct rtl8169_private *tp)
>  	return 0;
>  }
>  
> +static ssize_t
> +information_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	ssize_t ret;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	ret = rtl_dash_info(tp->rtl_dash, buf);
> +
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static DEVICE_ATTR_RO(information);
> +
> +static ssize_t
> +ap_ready_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	bool enable;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	enable = rtl_dash_get_ap_ready(tp->rtl_dash);
> +
> +	rtnl_unlock();
> +
> +	if (enable)
> +		strcat(buf, "enable\n");
> +	else
> +		strcat(buf, "disable\n");
> +
> +	return (strlen(buf) + 1);
> +}
> +
> +static ssize_t ap_ready_store(struct device *dev, struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	ssize_t len = strlen(buf);
> +	bool enable;
> +
> +	if (buf[len - 1] <= ' ')
> +		len--;
> +
> +	/* strlen("enable") = 6, and strlen("disable") = 7 */
> +	if (len != 6 && len != 7)
> +		return -EINVAL;
> +
> +	if (len == 6 && !strncmp(buf, "enable", 6))
> +		enable = true;
> +	else if (len == 7 && !strncmp(buf, "disable", 7))
> +		enable = false;
> +	else
> +		return -EINVAL;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	rtl_dash_set_ap_ready(tp->rtl_dash, enable);
> +
> +	rtnl_unlock();
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(ap_ready);
> +
> +static struct attribute *rtl_dash_attrs[] = {
> +	&dev_attr_ap_ready.attr,
> +	&dev_attr_information.attr,
> +	NULL
> +};
> +
> +static ssize_t cmac_data_write(struct file *fp, struct kobject *kobj,
> +			       struct bin_attribute *attr, char *buf,
> +			       loff_t offset, size_t size)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return -ENODEV;
> +
> +	if (size > CMAC_BUF_SIZE)
> +		return -EFBIG;
> +
> +	if (offset)
> +		return -EINVAL;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	ret = rtl_dash_to_fw(tp->rtl_dash, buf, size);
> +
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static ssize_t cmac_data_read(struct file *fp, struct kobject *kobj,
> +			      struct bin_attribute *attr, char *buf,
> +			      loff_t offset, size_t size)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +	int ret;
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return -ENODEV;
> +
> +	if (offset)
> +		return -EINVAL;
> +
> +	if (rtnl_lock_killable())
> +		return -EINTR;
> +
> +	ret = rtl_dash_from_fw(tp->rtl_dash, buf, size);
> +
> +	rtnl_unlock();
> +
> +	return ret;
> +}
> +
> +static BIN_ATTR_RW(cmac_data, CMAC_BUF_SIZE);
> +
> +static struct bin_attribute *rtl_dash_bin_attrs[] = {
> +	&bin_attr_cmac_data,
> +	NULL
> +};
> +
> +static umode_t is_dash_visible(struct kobject *kobj, struct attribute *attr,
> +			       int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return 0;
> +
> +	if (attr == &dev_attr_information.attr)
> +		return 0440;
> +
> +	return 0660;
> +}
> +
> +static umode_t is_dash_bin_visible(struct kobject *kobj,
> +				   struct bin_attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct net_device *net = to_net_dev(dev);
> +	struct rtl8169_private *tp = netdev_priv(net);
> +
> +	if (IS_ERR_OR_NULL(tp->rtl_dash))
> +		return 0;
> +
> +	return 0660;
> +}
> +
> +static struct attribute_group rtl_dash_grp = {
> +	.name = "dash",
> +	.is_visible	= is_dash_visible,
> +	.attrs		= rtl_dash_attrs,
> +	.is_bin_visible = is_dash_bin_visible,
> +	.bin_attrs = rtl_dash_bin_attrs,
> +};
> +
>  static void rtl8169_dash_release(struct rtl8169_private *tp)
>  {
>  	if (tp->dash_type > RTL_DASH_DP) {
> @@ -4717,6 +4895,8 @@ static int rtl8169_close(struct net_device *dev)
>  
>  	phy_disconnect(tp->phydev);
>  
> +	sysfs_remove_group(&dev->dev.kobj, &rtl_dash_grp);
> +
>  	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
>  			  tp->RxPhyAddr);
>  	dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
> @@ -4775,6 +4955,10 @@ static int rtl_open(struct net_device *dev)
>  		if (retval)
>  			goto err_release_fw_2;
>  
> +		retval = sysfs_create_group(&dev->dev.kobj, &rtl_dash_grp);
> +		if (retval < 0)
> +			goto err_release_dash;
> +
>  		tp->irq_mask |= DashCMAC | DashIntr;
>  	}
>  
> @@ -4782,7 +4966,7 @@ static int rtl_open(struct net_device *dev)
>  	retval = request_irq(pci_irq_vector(pdev, 0), rtl8169_interrupt,
>  			     irqflags, dev->name, tp);
>  	if (retval < 0)
> -		goto err_release_dash;
> +		goto err_remove_group;
>  
>  	retval = r8169_phy_connect(tp);
>  	if (retval)
> @@ -4798,6 +4982,8 @@ static int rtl_open(struct net_device *dev)
>  
>  err_free_irq:
>  	free_irq(pci_irq_vector(pdev, 0), tp);
> +err_remove_group:
> +	sysfs_remove_group(&dev->dev.kobj, &rtl_dash_grp);
>  err_release_dash:
>  	rtl8169_dash_release(tp);
>  err_release_fw_2:
> 

With regard to sysfs usage:
- attributes should be documented under /Documentation/ABI/testing
- attributes should be defined statically (driver.dev_groups instead
  of sysfs_create_group)
- for printing info there's sysfs_emit()
- is really RTNL needed? Or would a lighter mutex do?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ