Re: [PATCH net-next 1/8] devlink: Add generic parameter hw_tc_offload
Wed, Sep 12, 2018 at 08:34:22AM CEST, jakub.kicin...@netronome.com wrote:
>On Wed, 12 Sep 2018 11:47:59 +0530, Vasundhara Volam wrote:
>> On Tue, Sep 11, 2018 at 3:25 PM Jiri Pirko wrote:
>> >
>> > Tue, Sep 11, 2018 at 10:44:58AM CEST, vasundhara-v.vo...@broadcom.com
>> > wrote:
>> > >hw_tc_offload - Enable/Disable TC flower offload in the device.
>> > >
>> > >Signed-off-by: Vasundhara Volam
>> > >---
>> > > include/net/devlink.h | 4
>> > > net/core/devlink.c| 5 +
>> > > 2 files changed, 9 insertions(+)
>> > >
>> > >diff --git a/include/net/devlink.h b/include/net/devlink.h
>> > >index b9b89d6..a0e9ce9 100644
>> > >--- a/include/net/devlink.h
>> > >+++ b/include/net/devlink.h
>> > >@@ -362,6 +362,7 @@ enum devlink_param_generic_id {
>> > > DEVLINK_PARAM_GENERIC_ID_MAX_MACS,
>> > > DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
>> > > DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
>> > >+ DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
>> >
>> > Could you please describe why do you need this here and why the
>> > tc_offload flag in ethtool is not enough. How do you imagine the user
>> > should use them together?
>> Jiri, tc_offload flag in ethtool will modify feature in driver at
>> runtime. But I am adding
>> tc_offload param here to toggle this feature in NVM Config of our
>> adapter, whose configuration
>> mode is permanent and will be effective only with a reboot of the server.
>>
>> User has to turn on tc_offload feature in NVM config of the adapter and then
>> enabling the tc_offload flag in ethtool will completely enable the
>> feature in driver.
>
>Thanks for explaining, however, I don't think we have trouble
>understanding *what* you are doing, but rather *why* you're doing it.
Exactly. Why ethtool tc_offload flag is not enough?
Re: [PATCH net-next 1/8] devlink: Add generic parameter hw_tc_offload
On Wed, 12 Sep 2018 11:47:59 +0530, Vasundhara Volam wrote:
> On Tue, Sep 11, 2018 at 3:25 PM Jiri Pirko wrote:
> >
> > Tue, Sep 11, 2018 at 10:44:58AM CEST, vasundhara-v.vo...@broadcom.com
> > wrote:
> > >hw_tc_offload - Enable/Disable TC flower offload in the device.
> > >
> > >Signed-off-by: Vasundhara Volam
> > >---
> > > include/net/devlink.h | 4
> > > net/core/devlink.c| 5 +
> > > 2 files changed, 9 insertions(+)
> > >
> > >diff --git a/include/net/devlink.h b/include/net/devlink.h
> > >index b9b89d6..a0e9ce9 100644
> > >--- a/include/net/devlink.h
> > >+++ b/include/net/devlink.h
> > >@@ -362,6 +362,7 @@ enum devlink_param_generic_id {
> > > DEVLINK_PARAM_GENERIC_ID_MAX_MACS,
> > > DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
> > > DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
> > >+ DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
> >
> > Could you please describe why do you need this here and why the
> > tc_offload flag in ethtool is not enough. How do you imagine the user
> > should use them together?
> Jiri, tc_offload flag in ethtool will modify feature in driver at
> runtime. But I am adding
> tc_offload param here to toggle this feature in NVM Config of our
> adapter, whose configuration
> mode is permanent and will be effective only with a reboot of the server.
>
> User has to turn on tc_offload feature in NVM config of the adapter and then
> enabling the tc_offload flag in ethtool will completely enable the
> feature in driver.
Thanks for explaining, however, I don't think we have trouble
understanding *what* you are doing, but rather *why* you're doing it.
Re: [PATCH net-next 1/8] devlink: Add generic parameter hw_tc_offload
On Tue, Sep 11, 2018 at 3:25 PM Jiri Pirko wrote:
>
> Tue, Sep 11, 2018 at 10:44:58AM CEST, vasundhara-v.vo...@broadcom.com wrote:
> >hw_tc_offload - Enable/Disable TC flower offload in the device.
> >
> >Signed-off-by: Vasundhara Volam
> >---
> > include/net/devlink.h | 4
> > net/core/devlink.c| 5 +
> > 2 files changed, 9 insertions(+)
> >
> >diff --git a/include/net/devlink.h b/include/net/devlink.h
> >index b9b89d6..a0e9ce9 100644
> >--- a/include/net/devlink.h
> >+++ b/include/net/devlink.h
> >@@ -362,6 +362,7 @@ enum devlink_param_generic_id {
> > DEVLINK_PARAM_GENERIC_ID_MAX_MACS,
> > DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
> > DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
> >+ DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
>
> Could you please describe why do you need this here and why the
> tc_offload flag in ethtool is not enough. How do you imagine the user
> should use them together?
Jiri, tc_offload flag in ethtool will modify feature in driver at
runtime. But I am adding
tc_offload param here to toggle this feature in NVM Config of our
adapter, whose configuration
mode is permanent and will be effective only with a reboot of the server.
User has to turn on tc_offload feature in NVM config of the adapter and then
enabling the tc_offload flag in ethtool will completely enable the
feature in driver.
Re: [PATCH net-next 1/8] devlink: Add generic parameter hw_tc_offload
Tue, Sep 11, 2018 at 10:44:58AM CEST, vasundhara-v.vo...@broadcom.com wrote:
>hw_tc_offload - Enable/Disable TC flower offload in the device.
>
>Signed-off-by: Vasundhara Volam
>---
> include/net/devlink.h | 4
> net/core/devlink.c| 5 +
> 2 files changed, 9 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index b9b89d6..a0e9ce9 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -362,6 +362,7 @@ enum devlink_param_generic_id {
> DEVLINK_PARAM_GENERIC_ID_MAX_MACS,
> DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
> DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
>+ DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
Could you please describe why do you need this here and why the
tc_offload flag in ethtool is not enough. How do you imagine the user
should use them together?
[PATCH net-next 1/8] devlink: Add generic parameter hw_tc_offload
hw_tc_offload - Enable/Disable TC flower offload in the device.
Signed-off-by: Vasundhara Volam
---
include/net/devlink.h | 4
net/core/devlink.c| 5 +
2 files changed, 9 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b9b89d6..a0e9ce9 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -362,6 +362,7 @@ enum devlink_param_generic_id {
DEVLINK_PARAM_GENERIC_ID_MAX_MACS,
DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV,
DEVLINK_PARAM_GENERIC_ID_REGION_SNAPSHOT,
+ DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
/* add new param generic ids above here*/
__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -380,6 +381,9 @@ enum devlink_param_generic_id {
#define DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_NAME "region_snapshot_enable"
#define DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_TYPE DEVLINK_PARAM_TYPE_BOOL
+#define DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_NAME "hw_tc_offload"
+#define DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_TYPE DEVLINK_PARAM_TYPE_BOOL
+
#define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate) \
{ \
.id = DEVLINK_PARAM_GENERIC_ID_##_id, \
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 65fc366..020daa1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2676,6 +2676,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb,
struct genl_info *info)
.name = DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_NAME,
.type = DEVLINK_PARAM_GENERIC_REGION_SNAPSHOT_TYPE,
},
+ {
+ .id = DEVLINK_PARAM_GENERIC_ID_HW_TC_OFFLOAD,
+ .name = DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_NAME,
+ .type = DEVLINK_PARAM_GENERIC_HW_TC_OFFLOAD_TYPE,
+ },
};
static int devlink_param_generic_verify(const struct devlink_param *param)
--
1.8.3.1
