netdev - Re: [PATCH net-next v2] extend taskstats API to support networking accounts

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: <4F4700E1.3010407@gmail.com>
Date:	Fri, 24 Feb 2012 11:15:45 +0800
From:	Li Yu <raise.sail@...il.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
CC:	netdev@...r.kernel.org, David Miller <davem@...emloft.net>
Subject: Re: [PATCH net-next v2] extend taskstats API to support networking
 accounts

于 2012年02月24日 11:05, Ben Hutchings 写道:
> On Fri, 2012-02-24 at 10:43 +0800, Li Yu wrote:
>> This patch adds L7 traffic accounting in taskstats API, so
>> the iotop like applications can receive these statistics data.
>> In fact, I also have an iotop patch for this change.
>>
>> It ignores any protocol header overhead, so results of this
>> patch should be saw as the applications-aware data statistics
>> instead of traffic statistics on wire.
>>
>> The changes of v2 are:
>>
>> 1. Move up accounting source code, so they are not protocol-aware now.
>> 2. Skipping interrupt context accounting.
> [...]
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -1735,6 +1735,18 @@ static inline int skb_copy_to_page(struct sock
>> *sk, char __user *from,
>>    	return 0;
>>    }
>>
>> +static inline void task_net_accounting_rx(unsigned int len)
>> +{
>> +	if (!in_irq()&&  len>  0)
>> +		current->rx_bytes += len;
>> +}
>> +
>> +static inline void task_net_accounting_tx(unsigned int len)
>> +{
>> +	if (!in_irq()&&  len>  0)
>> +		current->tx_bytes += len;
>> +}
> [...]
>
> These are only called from system calls, so why are you checking for IRQ
> context?
>

Some caller are exported symbols or in exported symbols path, so
I think that we'd best that do not assume that such symbols only
work at process context, it may be used by some kernel modules in
softirq context.

However, faint, the 'len' should be signed int type.


>> @@ -558,7 +559,10 @@ static inline int __sock_sendmsg_nosec(struct kiocb
>> *iocb, struct socket *sock,
>>    	si->msg = msg;
>>    	si->size = size;
>>
>> -	return sock->ops->sendmsg(iocb, sock, msg, size);
>> +	ret = sock->ops->sendmsg(iocb, sock, msg, size);
>> +	task_net_accounting_tx(ret);
>> +
>> +	return ret;
>>    }
> [...]
>
> So what happens to the totals when sendmsg() returns an error?
>
> It seems to me that the parameter type for the accounting functions
> should be ssize_t.
>

En, I think that "int" is enough here at least, the return type of 
__sock_sendmsg_nosec() also is int ...

Thanks

Yu

> Ben.
>

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