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