[net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[John Fastabend]

Remove redundant DCB_CAP_DCBX_STATIC bit in DCB capabilities

Setting this bit indicates that no embedded DCBx engine is
present and the hardware can not be configured. This is the
same as having none of the DCB capability flags set or simply
not implementing the dcbnl ops at all.

This patch removes this bit. The bit has not made a stable
release yet so removing it should not be an issue with
existing apps.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
CC: Shmulik Ravid <shmulikr@broadcom.com>
---

 include/linux/dcbnl.h |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/include/linux/dcbnl.h b/include/linux/dcbnl.h
index 68cd248..ac9df34 100644
--- a/include/linux/dcbnl.h
+++ b/include/linux/dcbnl.h
@@ -423,18 +423,11 @@ enum dcbnl_cap_attrs {
  *
  * @DCB_CAP_DCBX_VER_IEEE: for a non-host DCBX engine, indicates the engine
  *                         supports the IEEE protocol flavor
- *
- * @DCB_CAP_DCBX_STATIC: for a non-host DCBX engine, indicates the engine
- *                       supports static configuration (i.e no actual
- *                       negotiation is performed negotiated parameters equal
- *                       the initial configuration)
- *
  */
 #define DCB_CAP_DCBX_HOST		0x01
 #define DCB_CAP_DCBX_LLD_MANAGED	0x02
 #define DCB_CAP_DCBX_VER_CEE		0x04
 #define DCB_CAP_DCBX_VER_IEEE		0x08
-#define DCB_CAP_DCBX_STATIC		0x10
 
 /**
  * enum dcbnl_numtcs_attrs - number of traffic classes

[net-2.6 PATCH 2/2] dcbnl: make get_app handling symmetric for IEEE and CEE DCBx

[John Fastabend]

The IEEE get/set app handlers use generic routines and do not
require the net_device to implement the dcbnl_ops routines. This
patch makes it symmetric so user space and drivers do not have
to handle the CEE version and IEEE DCBx versions differently.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/dcb/dcbnl.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index d900ab9..6b03f56 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -583,7 +583,7 @@ static int dcbnl_getapp(struct net_device *netdev, struct nlattr **tb,
 	u8 up, idtype;
 	int ret = -EINVAL;
 
-	if (!tb[DCB_ATTR_APP] || !netdev->dcbnl_ops->getapp)
+	if (!tb[DCB_ATTR_APP])
 		goto out;
 
 	ret = nla_parse_nested(app_tb, DCB_APP_ATTR_MAX, tb[DCB_ATTR_APP],
@@ -604,7 +604,16 @@ static int dcbnl_getapp(struct net_device *netdev, struct nlattr **tb,
 		goto out;
 
 	id = nla_get_u16(app_tb[DCB_APP_ATTR_ID]);
-	up = netdev->dcbnl_ops->getapp(netdev, idtype, id);
+
+	if (netdev->dcbnl_ops->getapp) {
+		up = netdev->dcbnl_ops->getapp(netdev, idtype, id);
+	} else {
+		struct dcb_app app = {
+					.selector = idtype,
+					.protocol = id,
+				     };
+		up = dcb_getapp(netdev, &app);
+	}
 
 	/* send this back */
 	dcbnl_skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);

Re: [net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[John Fastabend]

On 1/21/2011 6:35 PM, John Fastabend wrote:
> Remove redundant DCB_CAP_DCBX_STATIC bit in DCB capabilities
> 
> Setting this bit indicates that no embedded DCBx engine is
> present and the hardware can not be configured. This is the
> same as having none of the DCB capability flags set or simply
> not implementing the dcbnl ops at all.
> 
> This patch removes this bit. The bit has not made a stable
> release yet so removing it should not be an issue with
> existing apps.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> CC: Shmulik Ravid <shmulikr@broadcom.com>
> ---
> 

Shmulik, could you ACK this because you added these bits? But
I was adding support for this in lldpad and I see no reason that
we need these?

Thanks,
John.

Re: [net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[Shmulik Ravid]

On Fri, 2011-01-21 at 18:52 -0800, John Fastabend wrote:
> On 1/21/2011 6:35 PM, John Fastabend wrote:
> > Remove redundant DCB_CAP_DCBX_STATIC bit in DCB capabilities
> > 
> > Setting this bit indicates that no embedded DCBx engine is
> > present and the hardware can not be configured. This is the
> > same as having none of the DCB capability flags set or simply
> > not implementing the dcbnl ops at all.
> > 
> > This patch removes this bit. The bit has not made a stable
> > release yet so removing it should not be an issue with
> > existing apps.
> > 
> > Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> > CC: Shmulik Ravid <shmulikr@broadcom.com>
> > ---
> > 
> 
> Shmulik, could you ACK this because you added these bits? But
> I was adding support for this in lldpad and I see no reason that
> we need these?
> 
DCB_CAP_DCBX_STATIC means that the embedded engine will turn the user
configuration into the operational configuration without performing the
actual negotiation, so it is not equivalent to not having an embedded
DCBx engine. This is mostly a debug and integration option as it allows
you to do DCB related or dependent testing and development without
having a proper DCBx peer.

On second thought, I'm not sure this option is justified although we
found it useful during our development. If you think it's not useful
enough (or not at all) then by all means remove it.

Thanks,
Shmulik

Re: [net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[John Fastabend]

On 1/23/2011 8:53 AM, Shmulik Ravid wrote:
> 
> On Fri, 2011-01-21 at 18:52 -0800, John Fastabend wrote:
>> On 1/21/2011 6:35 PM, John Fastabend wrote:
>>> Remove redundant DCB_CAP_DCBX_STATIC bit in DCB capabilities
>>>
>>> Setting this bit indicates that no embedded DCBx engine is
>>> present and the hardware can not be configured. This is the
>>> same as having none of the DCB capability flags set or simply
>>> not implementing the dcbnl ops at all.
>>>
>>> This patch removes this bit. The bit has not made a stable
>>> release yet so removing it should not be an issue with
>>> existing apps.
>>>
>>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>>> CC: Shmulik Ravid <shmulikr@broadcom.com>
>>> ---
>>>
>>
>> Shmulik, could you ACK this because you added these bits? But
>> I was adding support for this in lldpad and I see no reason that
>> we need these?
>>
> DCB_CAP_DCBX_STATIC means that the embedded engine will turn the user
> configuration into the operational configuration without performing the
> actual negotiation, so it is not equivalent to not having an embedded
> DCBx engine. This is mostly a debug and integration option as it allows
> you to do DCB related or dependent testing and development without
> having a proper DCBx peer.
> 
> On second thought, I'm not sure this option is justified although we
> found it useful during our development. If you think it's not useful
> enough (or not at all) then by all means remove it.

We have an advertise bit in userspace that can be set and cleared to
do something similar for host based agents. I think for pg and application
data you can get the same behavior by setting the device to not willing.

However for PFC it could potentially be useful. But how would the
user set this mode? This is a capabilities bit indicating the device
supports this. Is there a way to subsequently put the device in this
mode?

--John  


> 
> Thanks,
> Shmulik
> 
> 
> 

Re: [net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[Shmulik Ravid]

On Sun, 2011-01-23 at 21:46 -0800, John Fastabend wrote:
> On 1/23/2011 8:53 AM, Shmulik Ravid wrote:
> > 
> > On Fri, 2011-01-21 at 18:52 -0800, John Fastabend wrote:
> >> On 1/21/2011 6:35 PM, John Fastabend wrote:
> >>> Remove redundant DCB_CAP_DCBX_STATIC bit in DCB capabilities
> >>>
> >>> Setting this bit indicates that no embedded DCBx engine is
> >>> present and the hardware can not be configured. This is the
> >>> same as having none of the DCB capability flags set or simply
> >>> not implementing the dcbnl ops at all.
> >>>
> >>> This patch removes this bit. The bit has not made a stable
> >>> release yet so removing it should not be an issue with
> >>> existing apps.
> >>>
> >>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> >>> CC: Shmulik Ravid <shmulikr@broadcom.com>
> >>> ---
> >>>
> >>
> >> Shmulik, could you ACK this because you added these bits? But
> >> I was adding support for this in lldpad and I see no reason that
> >> we need these?
> >>
> > DCB_CAP_DCBX_STATIC means that the embedded engine will turn the user
> > configuration into the operational configuration without performing the
> > actual negotiation, so it is not equivalent to not having an embedded
> > DCBx engine. This is mostly a debug and integration option as it allows
> > you to do DCB related or dependent testing and development without
> > having a proper DCBx peer.
> > 
> > On second thought, I'm not sure this option is justified although we
> > found it useful during our development. If you think it's not useful
> > enough (or not at all) then by all means remove it.
> 
> We have an advertise bit in userspace that can be set and cleared to
> do something similar for host based agents. I think for pg and application
> data you can get the same behavior by setting the device to not willing.
> 
True, but this requires a proper DCBx peer. The STATIC option is a bit
stronger.

> However for PFC it could potentially be useful. But how would the
> user set this mode? This is a capabilities bit indicating the device
> supports this. Is there a way to subsequently put the device in this
> mode?
You can set this mode by specifying this attribute in the set_dcbx
operation. The input to set_dcbx should be a subset of the advertised
dcbx attributes.

Shmulik 

Re: [net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[John Fastabend]

On 1/24/2011 7:27 AM, Shmulik Ravid wrote:
> 
> On Sun, 2011-01-23 at 21:46 -0800, John Fastabend wrote:
>> On 1/23/2011 8:53 AM, Shmulik Ravid wrote:
>>>
>>> On Fri, 2011-01-21 at 18:52 -0800, John Fastabend wrote:
>>>> On 1/21/2011 6:35 PM, John Fastabend wrote:
>>>>> Remove redundant DCB_CAP_DCBX_STATIC bit in DCB capabilities
>>>>>
>>>>> Setting this bit indicates that no embedded DCBx engine is
>>>>> present and the hardware can not be configured. This is the
>>>>> same as having none of the DCB capability flags set or simply
>>>>> not implementing the dcbnl ops at all.
>>>>>
>>>>> This patch removes this bit. The bit has not made a stable
>>>>> release yet so removing it should not be an issue with
>>>>> existing apps.
>>>>>
>>>>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>>>>> CC: Shmulik Ravid <shmulikr@broadcom.com>
>>>>> ---
>>>>>
>>>>

Dave, Please drop this patch sorry for the noise.

[...]

>> We have an advertise bit in userspace that can be set and cleared to
>> do something similar for host based agents. I think for pg and application
>> data you can get the same behavior by setting the device to not willing.
>>
> True, but this requires a proper DCBx peer. The STATIC option is a bit
> stronger.

At least in the PG case the CEE spec says the local configuration should be
used[1]. Application is a bit more vague in my opinion[2].

> 
>> However for PFC it could potentially be useful. But how would the
>> user set this mode? This is a capabilities bit indicating the device
>> supports this. Is there a way to subsequently put the device in this
>> mode?
> You can set this mode by specifying this attribute in the set_dcbx
> operation. The input to set_dcbx should be a subset of the advertised
> dcbx attributes.
> 

OK This works for me Shmulik thanks for the explanation.

[1] 3.1.4. http://www.ieee802.org/1/files/public/docs2008/az-wadekar-dcbx-capability-exchange-discovery-protocol-1108-v1.01.pdf

[2] 3.3.2. http://www.ieee802.org/1/files/public/docs2008/az-wadekar-dcbx-capability-exchange-discovery-protocol-1108-v1.01.pdf

Re: [net-2.6 PATCH 1/2] net: dcbnl: remove redundant DCB_CAP_DCBX_STATIC bit

[David Miller]

From: John Fastabend <john.r.fastabend@intel.com>
Date: Mon, 24 Jan 2011 07:52:48 -0800

> On 1/24/2011 7:27 AM, Shmulik Ravid wrote:
>> 
>> On Sun, 2011-01-23 at 21:46 -0800, John Fastabend wrote:
>>> On 1/23/2011 8:53 AM, Shmulik Ravid wrote:
>>>>
>>>> On Fri, 2011-01-21 at 18:52 -0800, John Fastabend wrote:
>>>>> On 1/21/2011 6:35 PM, John Fastabend wrote:
>>>>>> Remove redundant DCB_CAP_DCBX_STATIC bit in DCB capabilities
>>>>>>
>>>>>> Setting this bit indicates that no embedded DCBx engine is
>>>>>> present and the hardware can not be configured. This is the
>>>>>> same as having none of the DCB capability flags set or simply
>>>>>> not implementing the dcbnl ops at all.
>>>>>>
>>>>>> This patch removes this bit. The bit has not made a stable
>>>>>> release yet so removing it should not be an issue with
>>>>>> existing apps.
>>>>>>
>>>>>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>>>>>> CC: Shmulik Ravid <shmulikr@broadcom.com>
>>>>>> ---
>>>>>>
>>>>>
> 
> Dave, Please drop this patch sorry for the noise.

OK.

Re: [net-2.6 PATCH 2/2] dcbnl: make get_app handling symmetric for IEEE and CEE DCBx

[David Miller]

From: John Fastabend <john.r.fastabend@intel.com>
Date: Fri, 21 Jan 2011 18:35:18 -0800

> The IEEE get/set app handlers use generic routines and do not
> require the net_device to implement the dcbnl_ops routines. This
> patch makes it symmetric so user space and drivers do not have
> to handle the CEE version and IEEE DCBx versions differently.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

Applied, thanks John.