netdev - Re: [RFC PATCH v3 1/8] CAIF Protocol Stack

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]

Message-ID: <4B1EE474.5000508@babic.homelinux.org>
Date:	Wed, 09 Dec 2009 00:42:44 +0100
From:	stefano babic <stefano.babic@...ic.homelinux.org>
To:	sjur.brandeland@...ricsson.com
CC:	netdev@...r.kernel.org, randy.dunlap@...cle.com,
	kim.xx.lilliestierna@...ricsson.com,
	christian.bejram@...ricsson.com, daniel.martensson@...ricsson.com
Subject: Re: [RFC PATCH v3  1/8] CAIF Protocol Stack

sjur.brandeland@...ricsson.com wrote:

> +#define CAIF_PRIO_UNSPCEIFIED  0x0

The define is not used in code, but probably you would prefer to set
CAIF_PRIO_UNSPECIFIED


> diff --git a/include/linux/caif/caif_ioctl.h b/include/linux/caif/caif_ioctl.h
> +/*!\page  caif_ioctl.h
> + * This file defines the management interface to CAIF.
> + * It defines how CAIF channels are configured and become visible in the
> + * Linux file system, typically under "/dev".

This is not updated, you have already removed the char devices.

> + *
> + *\b Example - creating a new AT character device:
> + * \code
> +   fd = open("/dev/caifconfig",..);
> +   struct caif_channel_create_action at_config = {
> +	 .name = "cnhl2",
> +	 .config = {
> +	    .channel = CAIF_CHTY_AT,
> +	    .phy_ref = CAIF_PHY_LOW_LAT,
> +	    .priority = CAIF_PRIO_HIGH
> +	 }};
> +   ioctl(fd, CAIF_IOC_CONFIG_DEVICE,&at_config);
> +   close(fd);
> + * \endcode
> + * This will cause a new AT channel to be available in at "/dev/chnl2".
> + * This CAIF channel can then be connected by using \ref open.
> + */

This is obsolete,too.

> +/* Specifies the type of device to create: NET device or CHAR device*/
> +enum caif_dev_type {
> +	CAIF_DEV_CHR = 1,
> +	CAIF_DEV_NET = 2
> +};

You can remove this enum, it is not used anymore in your code.

> +
> +/** Used for identifying devices, PHY interfaces, etc.*/
> +struct caif_device_name {
> +	char name[DEVICE_NAME_LEN];	/*!< Device name */
> +	enum caif_dev_type devtype;	/*!< Device type */
> +};

I think this one is obsolete, too.

> +/* TODO: Move these defs to /usr/include/linux/socket.h */
> +#define AF_CAIF    37 		/* CAIF Socket Address Family */
> +#define PF_CAIF    37           /* CAIF Socket Protocol Family */
> +#define SOL_CAIF   278		/* CAIF Socket Option Level */

You sent already a patch for socket.h, I think you can safely remove
these duplicates.

> +
> +#define CAIF_DEFAULT_PRI       0xf

You have already defined CAIF_PRIO_NORMAL with the same value. Do you
need both ?

> +enum caif_link_selector {
> +	CAIF_LINK_UNSPECIFIED	= 0x00,
> +	CAIF_LINK_LOW_LATENCY	= 0xd0,
> +	CAIF_LINK_HIGH_BANDW	= 0xe0

caif_phy_preference and caif_link_selector have the same values for
priority. Should we have a single enum for both ?

Stefano

-- 
stefano <stefano.babic@...ic.homelinux.org>
GPG Key: 0x55814DDE
Fingerprint 4E85 2A66 4CBA 497A 2A7B D3BF 5973 F216 5581 4DDE
--
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