On 05/28/2017 02:03 AM, Or Gerlitz wrote:
On Sun, May 28, 2017 at 5:23 AM, Jes Sorensen <jes.soren...@gmail.com> wrote:
On 05/27/2017 05:02 PM, Or Gerlitz wrote:
On Sat, May 27, 2017 at 12:16 AM, Jes Sorensen <jes.soren...@gmail.com>
wrote:
This gets rid of the temporary #ifdef spaghetti and allows the code to
compile without offload support enabled.
I am pretty sure we can do that exercise you're up to without any
spaghetti cooking and even put more code under that CONFIG directive
(en_rep.c), I'll take that with Saeed.
I want to avoid adding #ifdef CONFIG_foo to the main code in order to keep
it readable. I did it gradually to make sure I didn't break anything and to
allow for it to be bisected in case something did break. If we can move out
more code from places like en_rep.c into eswitch_offload.c and get it
disabled that way that would be great, but I like to limit the number of
#ifdefs we add to the actual code.
FWIW (see below), squashing your seven patches to one resulted in a
fairly simple/clear
patch, so if we go that way, no need to have seven commits just for this piece.
Squashing patches into jumbo patches is inherently broken and bad coding
practice! It makes it way more complicated to debug and bisect in case a
minor detail broke in the process.
Just wondering, you are motivated by a wish to put some mlx5
functionalities under their own CONFIG directives which could be
useful when backporting the latest upstream driver into older kernel
and being able not to deal with parts of it, right? in that respect,
are you using SRIOV but not the offloads mode?
The motivation is two-fold, the primary is to be able to disable features
not being used for those who compile a custom kernel and who wish to reduce
the codebase compiled. It also makes it more flexible when back porting the
code to older kernels since it is easier to pick out a smaller subset. I was
going to look into making TC support etc. optional next, but I wanted to
have a discussion about this patchset first.
OKay, I got you.
Re SRIOV, I don't think it would be correct to break the support info few
CONFIG directives. If we want to allow someone to build the driver w.o
SRIOV that's fine, but I don't think we should further go down and disable
some of the SRIOV sub-modes.
Re TC offload support, that's make sense.
OK, so disabling SRIOV and disabling TC makes sense - I'll look at that.
Jes
