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] [day] [month] [year] [list]
Date:	Tue, 17 Mar 2015 10:11:54 -0600
From:	Philip Prindeville <philipp@...fish-solutions.com>
To:	Jiri Pirko <jiri@...nulli.us>
CC:	David Ahern <dsahern@...il.com>, netdev@...r.kernel.org,
	Scott Feldman <sfeldma@...il.com>
Subject: Re: [PATCH v2 2/3] rocker: add support for phys_port_name

Inline

On 03/17/2015 01:08 AM, Jiri Pirko wrote:
> Tue, Mar 17, 2015 at 05:27:25AM CET, philipp@...fish-solutions.com wrote:
>> Inline
>>
>> On 03/16/2015 09:27 PM, David Ahern wrote:
>>> Implement the phys_port_name operation. Port names are pulled from the
>>> rocker hardware model in qemu and default to the qemu name + port id.
>>> e.g.,
>>>
>>> sw1p1: flags=4098<BROADCAST,MULTICAST>  mtu 1500
>>>         ether 52:54:00:12:35:01  txqueuelen 1000  (Ethernet)
>>>         RX packets 0  bytes 0 (0.0 B)
>>>         RX errors 0  dropped 0  overruns 0  frame 0
>>>         TX packets 0  bytes 0 (0.0 B)
>>>         TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
>>>
>>> where 'sw1' comes from the qemu command line -device rocker,name=sw1, and
>>> 'p1' is port 1.
>>>
>>> Patch is adapted from Scott's phys_port_id patch.
>>>
>>> v2:
>>> - updated per changes in prior patch requested by Jiri
>>>
>>> Signed-off-by: David Ahern <dsahern@...il.com>
>>> Cc: Scott Feldman <sfeldma@...il.com>
>>> Cc: Jiri Pirko <jiri@...nulli.us>
>>> ---
>>>  drivers/net/ethernet/rocker/rocker.c | 64 ++++++++++++++++++++++++++++++++++++
>>>  drivers/net/ethernet/rocker/rocker.h |  1 +
>>>  2 files changed, 65 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>>> index 2511ae22ccd8..e201e7ef9bcf 100644
>>> --- a/drivers/net/ethernet/rocker/rocker.c
>>> +++ b/drivers/net/ethernet/rocker/rocker.c
>>> @@ -30,6 +30,7 @@
>>>  #include <linux/if_vlan.h>
>>>  #include <linux/if_bridge.h>
>>>  #include <linux/bitops.h>
>>> +#include <linux/ctype.h>
>>>  #include <net/switchdev.h>
>>>  #include <net/rtnetlink.h>
>>>  #include <net/ip_fib.h>
>>> @@ -1630,6 +1631,53 @@ rocker_cmd_get_port_settings_macaddr_proc(struct rocker *rocker,
>>>  	return 0;
>>>  }
>>>  
>>> +struct port_name {
>>> +	char *buf;
>>> +	int len;
>> I'd use "unsigned" for the len, since it's never going to be negative.
> It think the best is just to use "size_t" for len

Works for me.

-Philip

>>> +};
>>> +
>>> +static int
>>> +rocker_cmd_get_port_settings_phys_name_proc(struct rocker *rocker,
>>> +					    struct rocker_port *rocker_port,
>>> +					    struct rocker_desc_info *desc_info,
>>> +					    void *priv)
>>> +{
>>> +	struct rocker_tlv *info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_MAX + 1];
>>> +	struct rocker_tlv *attrs[ROCKER_TLV_CMD_MAX + 1];
>>> +	struct port_name *name = priv;
>>> +	struct rocker_tlv *attr;
>>> +	int i, j = 0, len;
>> I'd make 'i' and 'j' unsigned.
>>
>>
>>> +	char *str;
>>> +
>>> +	rocker_tlv_parse_desc(attrs, ROCKER_TLV_CMD_MAX, desc_info);
>>> +	if (!attrs[ROCKER_TLV_CMD_INFO])
>>> +		return -EIO;
>>> +
>>> +	rocker_tlv_parse_nested(info_attrs, ROCKER_TLV_CMD_PORT_SETTINGS_MAX,
>>> +				attrs[ROCKER_TLV_CMD_INFO]);
>>> +	attr = info_attrs[ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME];
>>> +	if (!attr)
>>> +		return -EIO;
>>> +
>>> +	len = min_t(int, rocker_tlv_len(attr), name->len);
>>> +	str = rocker_tlv_data(attr);
>>> +
>>> +	/* make sure name only contains alphanumeric characters */
>>> +	for (i = 0; i < len; ++i) {
>> Rather than hiding the initialization of 'j' above, I'd do:
>>
>> i = j = 0;
>>
>> instead.
>>
>>> +		if (isalnum(str[i])) {
>>> +			name->buf[j] = str[i];
>>> +			j++;
>>> +		}
>>> +	}
>>> +
>>> +	if (j == 0)
>>> +		return -EIO;
>>> +
>>> +	name->buf[j] = '\0';
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static int
>>>  rocker_cmd_set_port_settings_ethtool_prep(struct rocker *rocker,
>>>  					  struct rocker_port *rocker_port,
>>> @@ -4138,6 +4186,21 @@ static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>>>  				       rocker_port->brport_flags, mask);
>>>  }
>>>  
>>> +static int rocker_port_get_phys_port_name(struct net_device *dev,
>>> +					  char *buf, int len)
>> I'd use unsigned for 'len'.
>>
>>> +{
>>> +	struct rocker_port *rocker_port = netdev_priv(dev);
>>> +	struct port_name name = { .buf = buf, .len = len };
>>> +	int err;
>>> +
>>> +	err = rocker_cmd_exec(rocker_port->rocker, rocker_port,
>>> +			      rocker_cmd_get_port_settings_prep, NULL,
>>> +			      rocker_cmd_get_port_settings_phys_name_proc,
>>> +			      &name, false);
>>> +
>>> +	return err ? -EOPNOTSUPP : 0;
>>> +}
>>> +
>>>  static const struct net_device_ops rocker_port_netdev_ops = {
>>>  	.ndo_open			= rocker_port_open,
>>>  	.ndo_stop			= rocker_port_stop,
>>> @@ -4150,6 +4213,7 @@ static const struct net_device_ops rocker_port_netdev_ops = {
>>>  	.ndo_fdb_dump			= rocker_port_fdb_dump,
>>>  	.ndo_bridge_setlink		= rocker_port_bridge_setlink,
>>>  	.ndo_bridge_getlink		= rocker_port_bridge_getlink,
>>> +	.ndo_get_phys_port_name		= rocker_port_get_phys_port_name,
>>>  };
>>>  
>>>  /********************
>>> diff --git a/drivers/net/ethernet/rocker/rocker.h b/drivers/net/ethernet/rocker/rocker.h
>>> index 51e430d25138..a4e9591d7457 100644
>>> --- a/drivers/net/ethernet/rocker/rocker.h
>>> +++ b/drivers/net/ethernet/rocker/rocker.h
>>> @@ -158,6 +158,7 @@ enum {
>>>  	ROCKER_TLV_CMD_PORT_SETTINGS_MACADDR,		/* binary */
>>>  	ROCKER_TLV_CMD_PORT_SETTINGS_MODE,		/* u8 */
>>>  	ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING,		/* u8 */
>>> +	ROCKER_TLV_CMD_PORT_SETTINGS_PHYS_NAME,		/* binary */
>>>  
>>>  	__ROCKER_TLV_CMD_PORT_SETTINGS_MAX,
>>>  	ROCKER_TLV_CMD_PORT_SETTINGS_MAX =
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ