[net-next PATCH] net: dcb: add CEE notify calls

[net-next PATCH] net: dcb: add CEE notify calls

[John Fastabend]

This adds code to trigger CEE events when an APP change or setall
command is made from user space. This simplifies user space code
significantly by creating a single interface to listen on that
works with both firmware and userland agents.

And if we end up with multiple agents this keeps every thing in
sync userland agents, firmware agents, and kernel notifier consumers.

For an example agent that listens for these events see:

https://github.com/jrfastab/cgdcbxd

cgdcbxd is a daemon used to monitor DCB netlink events and manage
the net_prio control group sub-system.

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

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

diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index 8dfa1da..656c7c7 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -704,6 +704,7 @@ static int dcbnl_setapp(struct net_device *netdev, struct nlattr **tb,
 
 	ret = dcbnl_reply(err, RTM_SETDCB, DCB_CMD_SAPP, DCB_ATTR_APP,
 			  pid, seq, flags);
+	dcbnl_cee_notify(netdev, RTM_SETDCB, DCB_CMD_SAPP, seq, 0);
 out:
 	return ret;
 }
@@ -936,6 +937,7 @@ static int dcbnl_setall(struct net_device *netdev, struct nlattr **tb,
 
 	ret = dcbnl_reply(netdev->dcbnl_ops->setall(netdev), RTM_SETDCB,
 	                  DCB_CMD_SET_ALL, DCB_ATTR_SET_ALL, pid, seq, flags);
+	dcbnl_cee_notify(netdev, RTM_SETDCB, DCB_CMD_SET_ALL, seq, 0);
 
 	return ret;
 }

Re: [net-next PATCH] net: dcb: add CEE notify calls

[Shmulik Ravid]

> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> 
>  net/dcb/dcbnl.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index 8dfa1da..656c7c7 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -704,6 +704,7 @@ static int dcbnl_setapp(struct net_device *netdev, struct nlattr **tb,
>  
>  	ret = dcbnl_reply(err, RTM_SETDCB, DCB_CMD_SAPP, DCB_ATTR_APP,
>  			  pid, seq, flags);
> +	dcbnl_cee_notify(netdev, RTM_SETDCB, DCB_CMD_SAPP, seq, 0);
>  out:
>  	return ret;
>  }
> @@ -936,6 +937,7 @@ static int dcbnl_setall(struct net_device *netdev, struct nlattr **tb,
>  
>  	ret = dcbnl_reply(netdev->dcbnl_ops->setall(netdev), RTM_SETDCB,
>  	                  DCB_CMD_SET_ALL, DCB_ATTR_SET_ALL, pid, seq, flags);
> +	dcbnl_cee_notify(netdev, RTM_SETDCB, DCB_CMD_SET_ALL, seq, 0);
>  
>  	return ret;
>  }
> 

In case of a FW DCBx agent these notification could be a bit confusing.
In this case, the dcbnl_setxxx operations are used to set the initial
negotiation parameters. dcbnl_setall triggers the negotiation and some
time after that the FW DCBx agent driver should send a notification with
the newly negotiated values. The notifications sent form within the set
operations will return the current negotiated values which may very soon
change. If we want to keep the user mode code simple and unified maybe
we should send these notifications only if the DCBx agent is host based.

Shmulik 

Re: [net-next PATCH] net: dcb: add CEE notify calls

[John Fastabend]

On 4/23/2012 5:51 AM, Shmulik Ravid wrote:
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>> ---
>>
>>  net/dcb/dcbnl.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
>> index 8dfa1da..656c7c7 100644
>> --- a/net/dcb/dcbnl.c
>> +++ b/net/dcb/dcbnl.c
>> @@ -704,6 +704,7 @@ static int dcbnl_setapp(struct net_device *netdev, struct nlattr **tb,
>>  
>>  	ret = dcbnl_reply(err, RTM_SETDCB, DCB_CMD_SAPP, DCB_ATTR_APP,
>>  			  pid, seq, flags);
>> +	dcbnl_cee_notify(netdev, RTM_SETDCB, DCB_CMD_SAPP, seq, 0);
>>  out:
>>  	return ret;
>>  }
>> @@ -936,6 +937,7 @@ static int dcbnl_setall(struct net_device *netdev, struct nlattr **tb,
>>  
>>  	ret = dcbnl_reply(netdev->dcbnl_ops->setall(netdev), RTM_SETDCB,
>>  	                  DCB_CMD_SET_ALL, DCB_ATTR_SET_ALL, pid, seq, flags);
>> +	dcbnl_cee_notify(netdev, RTM_SETDCB, DCB_CMD_SET_ALL, seq, 0);
>>  
>>  	return ret;
>>  }
>>
> 
> In case of a FW DCBx agent these notification could be a bit confusing.
> In this case, the dcbnl_setxxx operations are used to set the initial
> negotiation parameters. dcbnl_setall triggers the negotiation and some
> time after that the FW DCBx agent driver should send a notification with
> the newly negotiated values. The notifications sent form within the set
> operations will return the current negotiated values which may very soon
> change. If we want to keep the user mode code simple and unified maybe
> we should send these notifications only if the DCBx agent is host based.
> 
> Shmulik 
> 

No. We want all the firmware agents and host based agents to look the
same from the application. The situation you described is exactly the
same for user space as in firmware. The DCBx state machine starts and may
call dcbnl_setxxx with initial (local) values. At some later point these
may change (possibly because of negotiation with a peer) and we need to
call dcbnl_setxxx again.

I don't see how this complicates any user mode code? Presumably the agent
is listening to DCBx events because it really wants to know the current
state of DCBx. It seems to me skipping notifications will actually cause
more issues this results in the hardware being in some state that did not
trigger any events and the agent will now be out of sync. This is the
problem I am trying to solve.

btw with this patch we can remove the notify calls in bnx2x.

.John

Re: [net-next PATCH] net: dcb: add CEE notify calls

[Shmulik Ravid]

> No. We want all the firmware agents and host based agents to look the
> same from the application. The situation you described is exactly the
> same for user space as in firmware. The DCBx state machine starts and may
> call dcbnl_setxxx with initial (local) values. At some later point these
> may change (possibly because of negotiation with a peer) and we need to
> call dcbnl_setxxx again.
> 
> I don't see how this complicates any user mode code? Presumably the agent
> is listening to DCBx events because it really wants to know the current
> state of DCBx. It seems to me skipping notifications will actually cause
> more issues this results in the hardware being in some state that did not
> trigger any events and the agent will now be out of sync. This is the
> problem I am trying to solve.
> 
> btw with this patch we can remove the notify calls in bnx2x.
> 
> .John
> 
OK, I see.
>From a user mode application monitoring the netlink notification you get
successive updates each indicating the current valid negotiated
parameters (and HW state) and that's fine.
However I don't see how you can remove the notification call form the
bnx2x. When the FW DCBx agent decides to change the negotiated
parameters (perhaps in response to a peer request), it alerts the driver
which configures the HW and then needs to somehow notify the user about
the newly negotiated parameters.

Shmulik  

Re: [net-next PATCH] net: dcb: add CEE notify calls

[John Fastabend]

On 4/23/2012 10:00 AM, Shmulik Ravid wrote:
> 
>> No. We want all the firmware agents and host based agents to look the
>> same from the application. The situation you described is exactly the
>> same for user space as in firmware. The DCBx state machine starts and may
>> call dcbnl_setxxx with initial (local) values. At some later point these
>> may change (possibly because of negotiation with a peer) and we need to
>> call dcbnl_setxxx again.
>>
>> I don't see how this complicates any user mode code? Presumably the agent
>> is listening to DCBx events because it really wants to know the current
>> state of DCBx. It seems to me skipping notifications will actually cause
>> more issues this results in the hardware being in some state that did not
>> trigger any events and the agent will now be out of sync. This is the
>> problem I am trying to solve.
>>
>> btw with this patch we can remove the notify calls in bnx2x.
>>
>> .John
>>
> OK, I see.
>>From a user mode application monitoring the netlink notification you get
> successive updates each indicating the current valid negotiated
> parameters (and HW state) and that's fine.
> However I don't see how you can remove the notification call form the
> bnx2x. When the FW DCBx agent decides to change the negotiated
> parameters (perhaps in response to a peer request), it alerts the driver
> which configures the HW and then needs to somehow notify the user about
> the newly negotiated parameters.
> 

You are right here. I guess only the notify call after setapp can be removed.
Can you ACK the patch to indicate this addresses your concerns?

.John

> Shmulik  
> 
> 

Re: [net-next PATCH] net: dcb: add CEE notify calls

[Shmulik Ravid]

On Fri, 2012-04-20 at 12:49 -0700, John Fastabend wrote:
> This adds code to trigger CEE events when an APP change or setall
> command is made from user space. This simplifies user space code
> significantly by creating a single interface to listen on that
> works with both firmware and userland agents.
> 
> And if we end up with multiple agents this keeps every thing in
> sync userland agents, firmware agents, and kernel notifier consumers.
> 
> For an example agent that listens for these events see:
> 
> https://github.com/jrfastab/cgdcbxd
> 
> cgdcbxd is a daemon used to monitor DCB netlink events and manage
> the net_prio control group sub-system.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> 

Acked-by: Shmulik Ravid <shmulikr@broadcom.com>

Thanks John,

Shmulik 

Re: [net-next PATCH] net: dcb: add CEE notify calls

[David Miller]

From: "Shmulik Ravid" <shmulikr@broadcom.com>
Date: Tue, 24 Apr 2012 15:56:19 +0300

> 
> On Fri, 2012-04-20 at 12:49 -0700, John Fastabend wrote:
>> This adds code to trigger CEE events when an APP change or setall
>> command is made from user space. This simplifies user space code
>> significantly by creating a single interface to listen on that
>> works with both firmware and userland agents.
>> 
>> And if we end up with multiple agents this keeps every thing in
>> sync userland agents, firmware agents, and kernel notifier consumers.
>> 
>> For an example agent that listens for these events see:
>> 
>> https://github.com/jrfastab/cgdcbxd
>> 
>> cgdcbxd is a daemon used to monitor DCB netlink events and manage
>> the net_prio control group sub-system.
>> 
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>> ---
>> 
> 
> Acked-by: Shmulik Ravid <shmulikr@broadcom.com>

Applied, thanks.