Hi Samuel,
ext Samuel Ortiz wrote:
> Hi Jukka,
>
> On Mon, Feb 21, 2011 at 03:38:20PM +0200, Jukka Rissanen wrote:
>
>> +/* Note that addattr32(), addattr_l(), rtnl_close(), rtnl_open() and
>> + * rtnl_talk() are from libnetlink.c that is found in iproute2
>> +package + * and copyright by Alexey Kuznetsov et al.
>> + */
> Ok, so it would probably make sense to add that to the Copyright.
Sure, will do that.
>
>> +static int rtnl_open(struct rtnl_handle *rth) {
>> + socklen_t addr_len;
>> + int sndbuf = 32768;
>> + int rcvbuf = 32768;
>> +
>> + memset(rth, 0, sizeof(*rth));
>> +
>> + rth->fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE); + if
>> (rth->fd < 0) { + DBG("Cannot open netlink socket: %s",
>> strerror(errno)); + return -1; + }
>> +
>> + if (setsockopt(rth->fd, SOL_SOCKET, SO_SNDBUF, &sndbuf,
>> + sizeof(sndbuf)) < 0) { + DBG("SO_SNDBUF:
>> %s", strerror(errno));
>> + return -1;
>> + }
>> +
>> + if (setsockopt(rth->fd, SOL_SOCKET, SO_RCVBUF, &rcvbuf,
>> + sizeof(rcvbuf)) < 0) { + DBG("SO_RCVBUF:
>> %s", strerror(errno));
>> + return -1;
>> + }
> So why do we need those big IO buffers ?
Yep, the buffer size need to be tweaked. I just copied the code from iproute
project and forgot to fix buffer size.
>> + memset(&rth->local, 0, sizeof(rth->local));
>> + rth->local.nl_family = AF_NETLINK;
>> + rth->local.nl_groups = 0;
>> +
>> + if (bind(rth->fd, (struct sockaddr *)&rth->local,
>> + sizeof(rth->local)) < 0) {
>> + DBG("Cannot bind netlink socket: %s", strerror(errno));
> This should be a connman_error().
> <picky>
> And I'd prefer a "Can not bind netlink socket %s" string.
> </picky>
Ok :)
>> +static int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
>> pid_t peer, + unsigned groups, struct nlmsghdr *answer)
> So it seems we only need rtnl_talk() to send an rtnl message and
> check that we do get an ACK from the kernel. So we could probably
> simplify this routine quite a bit.
True, I try to refactor it.
>> + char buf[16384];
> Same comment as above: We probably don't need that big of a buffer.
Yes
>> + status = sendmsg(rtnl->fd, &msg, 0);
> So for all the socket IO we need to use gio.
Ok
>> + while (1) {
>> + iov.iov_len = sizeof(buf);
>> + status = recvmsg(rtnl->fd, &msg, 0);
> As far as I understand, this is a blocking receive.
> Again, you need to use gio here and watch for data to arrive on your
> socket.
Yep, that needs to be changed.
>
>> +static int tunnel_up()
>> +{
>> +}
> I would like to see this one integrated with inet.c. So you probably
> need to add an MTU setting routine there and then you can use the
> existing APIs for tunnel_up().
Ok, that makes sense.
Jukka
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
