| 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
| ||
|
Message-ID: <5508524A.70807@redfish-solutions.com>
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