Re: [RFC PATCH 2/2] PID-based packet filtering support

[chetan loke <loke.chetan@gmail.com>]

On Wed, Apr 4, 2012 at 5:16 AM, Nuno Martins
<nuno.martins@caixamagica.pt> wrote:

>  net/pidmonitor/Makefile           |    3 +

might make sense to prefix files with 'net' tag. Like: net_pidmonitor,
net_proc_monitor.c etc. Because proc_monitor is too generic.


> +static int is_equal_packet_info(struct packetInfo *pi,

It should be 'packet_info' and not 'packetInfo'. Init-char convention
is not used in linux. same goes for portInfo and other structs(if
any).


> +static int insert_address(struct packetInfo *lpi, struct portInfo *port_info)
> +{
> +
> +       switch (lpi->protocol) {
> +       case IPPROTO_TCP:
> +               if (!(port_info->tcp)) {
> +                       port_info->tcp = kmalloc(sizeof(struct local_addresses_list), GFP_KERNEL);

Do you think it might make sense to pre-alloc a list? or
managing/growing that list would add more pain/code than it's worth?
I'm just thinking in terms of search scalability when memory is
fragmented and we have lots of nodes in the tree.But I'm sure you
must've done some light tests.


> +static struct portInfo *create_packet_info(struct packetInfo *lpi)
> +{
> +       struct portInfo *pi = NULL;
> +       pi = kmalloc(sizeof(*pi), GFP_KERNEL);
> +
> +       if (!pi)
> +               return NULL;
> +
> +       pi->port = lpi->port;
> +       pi->tcp = NULL;
> +       pi->tcp_list_counter = 0;
> +       pi->udp = NULL;
> +       pi->udp_list_counter = 0;
> +
> +       insert_address(lpi, pi);
> +
> +       return pi;
> +}
> +

Just thinking out loud - what if packet_info and pid were introduced
as part of struct sock or something? The no need to kmalloc.
And at the end of the bind/accept/other-calls-you-intercept-via-probes
you can populate the structs. Then the kprobes could go away too. But
I know nothing about BPF so I can't really comment for sure.


Chetan