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

Powered by blists - more mailing lists