netdev - Re: [PATCH] net: unisys: adding unisys virtnic driver

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: <wrfjd26o2c27.fsf@redhat.com>
Date:	Fri, 09 Jan 2015 10:44:48 -0500
From:	Jes Sorensen <Jes.Sorensen@...hat.com>
To:	Erik Arfvidson <earfvids@...hat.com>
Cc:	benjamin.romer@...sys.com, netdev@...r.kernel.org,
	dzickus@...hat.com, davem@...emloft.net, Bruce.Vessey@...sys.com,
	sparmaintainer@...sys.com, prarit@...hat.com,
	Neil Horman <nhorman@...hat.com>
Subject: Re: [PATCH] net: unisys: adding unisys virtnic driver

Erik Arfvidson <earfvids@...hat.com> writes:
> The purpose of this patch is to add Unisys virtual network driver
> into the network directory and also to start a discussion about
> the requirements needed.
>
> Signed-off-by: Erik Arfvidson <earfvids@...hat.com>

Erik,

I was discussing this with colleagues and I want to give you some
general comments on this. My comments are not specific to virtnic.c
itself.

Looking over the logs of drivers/staging/unisys, I noticed a fair amount
of cleanups has been applied, but not a lot of fixes addressing what I
would consider the real issues.

The first thing you should work on is to get rid of
drivers/staging/unisys/uislib - it looks to provide a lot of wrappers
and utility functions which already exist. You need to address things
like:

 - Custom atomic primitives (uisqueue.c)
 - List handlers (use list.h) and all the utility functions we provide
 - Functions for launching killing kernel threads (uisthread)
 - There is most of a bus implementation in there - is this really
   needed, ie. are the devices sitting on a PCI bus, or is this some
   special bus type?
 - Use proper data types - your code should contain no 'long long' ever!
   If you need data types of a specific size, use u8/u16/u32/u64, and
   please get rid of broken Windows stuff such as BOOL and #pragma.
 - /proc handlers - you should most likely be using /sys
   (configs/debugfs) and don't wrap things in libraries, do it near the
   code using it.

Basically I recommend you start working your way through uislib, and
once you have eliminated 90% of it, you should be a lot closer to code
that can go into mainline.

I know my colleague Neil has some issues on this specific driver, which
I have less insight into, so I think he'll post some comments on that
too.

I hope this is helpful!

Cheers,
Jes
--
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