netdev - Re: [PATCH net-next-2.6 v4 02/12] net-caif: add CAIF socket and configuration headers

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: <1267226115.18491.71.camel@violet>
Date:	Sat, 27 Feb 2010 00:15:15 +0100
From:	Marcel Holtmann <marcel@...tmann.org>
To:	sjur.brandeland@...ricsson.com
Cc:	netdev@...r.kernel.org, davem@...emloft.net,
	daniel.martensson@...ricsson.com, kaber@...sh.net,
	stefano.babic@...ic.homelinux.org, randy.dunlap@...cle.com
Subject: Re: [PATCH net-next-2.6 v4 02/12] net-caif: add CAIF socket and
 configuration headers

Hi Sjur,

I think most issues have been resolved and this should be ready for
merging, but I am bit worried about the userspace API. Can we start a
bit smaller and extend it later? Especially the socket options worry me
a bit.

Dave, personally I would prefer if we can merge this without these
socket options. Since I am really missing the need for it.

> +/**
> + * enum caif_socket_opts - CAIF option values for getsockopt and setsockopt.
> + *
> + * @CAIFSO_LINK_SELECT:		Selector used if multiple CAIF Link layers are
> + *				available. Either a high bandwidth
> + *				link can be selected (CAIF_LINK_HIGH_BANDW) or
> + *				or a low latency link (CAIF_LINK_LOW_LATENCY).
> + *                              This option is of type u_int32_t.
> + *				Alternatively SO_BINDTODEVICE can be used.
> + *
> + * @CAIFSO_REQ_PARAM:		Used to set the request parameters for a
> + *				utility channel. (struct caif_param). This
> + *				option must be set before connecting.
> + *
> + * @CAIFSO_RSP_PARAM:		Gets the request parameters for a utility
> + *				channel. (struct caif_param). This option
> + *				is valid after a successful connect.

These two more look like a combination of setsockopt/getsockopt instead
of two socket options. Maybe it is leftover from a ioctl interface, but
socket options work differently.

Also the caif_param struct seems pointless. Socket options contain a
length parameter anyway. So why bother with a struct that is just a data
field and a length field.

> + * @CAIFSO_CHANNEL_ID:		Gets the channel id on a CAIF Channel.
> + *				This option is valid after a successful connect.
> + *				( u_int32_t)

Where is this used and what is it used for? Is this something that
shouldn't be better part of the sockaddr structure. Then you can use
getpeername for it?

> + * @CAIFSO_NEXT_PAKCET_LEN:	Gets the size of next received packet.
> + *				Value is 0 if no packet is available.
> + *				This option is valid after a successful connect.
> + *				( u_int32_t)

Typo. And why do we need this?

> + * @CAIFSO_MAX_PAKCET_LEN:	Gets the maximum packet size for this
> + *				connection. ( u_int32_t)

Isn't this more like SO_RCVBUF or SO_SNDBUF.

Regards

Marcel

--
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