On Sun, Jan 29, 2017 at 12:07 AM, Saeed Mahameed <sae...@dev.mellanox.co.il> wrote: > On Sat, Jan 28, 2017 at 7:19 PM, Tom Herbert <t...@herbertland.com> wrote: >> On Sat, Jan 28, 2017 at 3:38 AM, Saeed Mahameed >> <sae...@dev.mellanox.co.il> wrote: >>> On Fri, Jan 27, 2017 at 8:13 PM, Tom Herbert <t...@herbertland.com> wrote: >>>> On Fri, Jan 27, 2017 at 9:58 AM, Saeed Mahameed >>>> <sae...@dev.mellanox.co.il> wrote: >>>>> On Fri, Jan 27, 2017 at 1:32 AM, Tom Herbert <t...@herbertland.com> wrote: >>>>>> This patchset creates configuration options for sriov, vxlan, eswitch, >>>>>> and tc features in the mlx5 driver. The purpose of this is to allow not >>>>>> building these features. These features are optional advanced features >>>>>> that are not required for a core Ethernet driver. A user can disable >>>>>> these features which resuces the amount of code in the driver. Disabling >>>>>> these features (and DCB) reduces the size of mlx5_core.o by about 16%. >>>>>> This is also can reduce the complexity of backport and rebases since >>>>>> user would no longer need to worry about dependencies with the rest of >>>>>> the kernel that features which might not be of any interest to a user >>>>>> may bring in. >>>>>> >>>>>> Tested: Build and ran the driver with all features enabled (the default) >>>>>> and with none enabled (including DCB). Did not see any issues. I did >>>>>> not explicity test operation of ayy of features in the list. >>>>>> >>>>> >>>>> Basically I am not against this kind of change, infact i am with it, >>>>> although I would have done some restructuring in the driver before i >>>>> did such change ;), filling the code with ifdefs is not a neat thing. >>>>> >>>> If you wish, please take this as an RFC and feel free to structure the >>>> code the right way. I think the intent is clear enough and looks like >>>> davem isn't going to allow the directory restructuring so something >>>> like this seems to be the best course of action now. >>>> >>> >>> Right. >>> >>>>> I agree this will simplify backporting and provide some kind of >>>>> feature separation inside the driver. >>>>> But this will also increase the testing matrix we need to cover and >>>>> increase the likelihood of kbuild breaks by an order of magnitude. >>>>> >>>> The testing matrix already exploded with the proliferation of >>>> supported features. If anything this reduces the test matrix problem. >>>> For instance, if we make a change to the core driver and functionality >>>> properly isolated there is a much better chance that this won't affect >>>> peripheral functionality and vice versa. It is just not feasible for >>>> us to test every combination of NIC features for every change being >>>> made. >>>> >>> >>> Yes for isolated features, but for base functionality, we need to test >>> it with all new device specific kconfig combinations on every patch! >> >> Sorry, but that is the price you need to pay for a feature rich device. >> >> On the subject of testing, I don't really see any indication in these >> patches on how patches are being tested. Also, there are patches that >> fix things without any mention of how to repro the problems. It is > > I Will do my best to provide more information in fixes commit > messages, Thanks for the input. > >> critical that we know IPv6 is tested as much or more than IPv4 (just > > For the record, for every IPv4 test in our automatic regression system > we have an IPv6 equivalent test, > not to mention IPv6-only directed tests. > Great to know. Is there a publicly available description of what specific tests are in the suite?
>> last week with hit yet another IPv6-only issue in an another upstream >> driver that should have been caught with a simple load test-- this > > Is there any IPv6-only functionality issues with mlx4/mlx5 that we > don't know about ? > If you do see any of those, please report them so we take the needed > corrective actions to fix them and cover them in our regression. > If we see them we will certainly let you know. This is not just functionality either, performance regression versus IPv4 should also be considered serious issues. >> really is not acceptable any more!). Please add a description of how >> patches were tested to commit logs. >> > > Acknowledge. > Thanks, Tom >> Tom >> >>> since a misplaced code inside or outside the correct ifdef >>> can easily go unnoticed and break functionality. >>> >>>>> One more thing, do we really need a device specific flag per feature >>>>> per vendor per device? can't we just use the same kconfig flag for >>>>> all drivers and if there is a more generic system wide flag that >>>>> covers the same feature >>>>> can't we just use it, for instance instead of >>>>> CONFIG_<DRIVER_NAME>_SRIOV why not use already existing CONFIG_PCI_IOV >>>>> for all drivers ? >>>>> >>>> That sounds good to me. We already have CONFIG_RFS_ACCEL and others >>>> that do that. >>>> >>>> Tom >>>> >>>>> Saeed. >>>>> >>>>>> >>>>>> >>>>>> Tom Herbert (4): >>>>>> mlx5: Make building eswitch configurable >>>>>> mlx5: Make building SR-IOV configurable >>>>>> mlx5: Make building tc hardware offload configurable >>>>>> mlx5: Make building vxlan hardware offload configurable >>>>>> >>>>>> drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 35 ++++++ >>>>>> drivers/net/ethernet/mellanox/mlx5/core/Makefile | 16 ++- >>>>>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 129 >>>>>> ++++++++++++++++------ >>>>>> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 39 +++++-- >>>>>> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 4 +- >>>>>> drivers/net/ethernet/mellanox/mlx5/core/lag.c | 2 + >>>>>> drivers/net/ethernet/mellanox/mlx5/core/main.c | 32 ++++-- >>>>>> drivers/net/ethernet/mellanox/mlx5/core/sriov.c | 6 +- >>>>>> 8 files changed, 205 insertions(+), 58 deletions(-) >>>>>> >>>>>> -- >>>>>> 2.9.3 >>>>>>
