[openib-general] [PATCH 8/10] sysfs interface implementation

[Bryan O'Sullivan]

Ramachandra K wrote:
> 
> +/*
> + * target eiocs are added by writing
> + *
> + * ioc_guid=<EIOC GUID>,dgid=<dest GID>,pkey=<P_key>,name=<interface_name>
> + * to the create_primary  sysfs attribute.
> + */
> +enum {
> +	VNIC_OPT_ERR = 0,
> +	VNIC_OPT_IOC_GUID = 1 << 0,
> +	VNIC_OPT_DGID = 1 << 1,
> +	VNIC_OPT_PKEY = 1 << 2,
> +	VNIC_OPT_NAME = 1 << 3,
> +	VNIC_OPT_INSTANCE = 1 << 4,
> +	VNIC_OPT_RXCSUM = 1 << 5,
> +	VNIC_OPT_TXCSUM = 1 << 6,
> +	VNIC_OPT_HEARTBEAT = 1 << 7,
> +	VNIC_OPT_ALL = (VNIC_OPT_IOC_GUID |
> +			VNIC_OPT_DGID | VNIC_OPT_NAME | VNIC_OPT_PKEY),
> +};

This is not OK.  You can't pass in multiple values to a sysfs file. 
Either set the values separately or (if they have to be set all at once) 
  find some other way to do this work.  Also, putting all of this 
parsing cruft in a driver is a sign you're trying to do something you 
shouldn't be.

> +static int avg_ticks_as_time(cycles_t ticks, u32 count, char *buffer)

Leave out all the pretty printing.  Just print a number in standard units,
and let userspace do the parsing.

> +static int setup_vnic_stats_files(struct vnic *vnic)
> +{

This code needs to use sysfs_create_group instead.

>
> +static int create_netpath(struct netpath *n_pdest, struct path_param *p_params)
> +{

Why does this not return any error values?


> +struct vnic *create_vnic(struct path_param *param)
> +{

Ditto with the sysfs_create_group.

	<b