MRP: listener merge call can cause incorrect listener declaration

[andrew-elder]

The following sequence has been observed

1. Listener end station client declares Listener Asking Failed (because Talker declaration has not yet been observed)
2. Listener end station observes Talker declaration and then declares Listener Ready. Listener's mrpd waits for a transmit opportunity
3. Switch sends rIn! to end station declaring Listener Asking Failed. Merge operation in https://github.com/AVnu/Open-AVB/blob/master/daemons/mrpd/msrp.c#L276 causes declaration to be changed from the desired Listener Ready to Listener Asking Failed.

Captured logs for 5 streams follow (state 1 is Listener Asking Failed and state 2 is Listener Ready)

Listener declares Listener Asking Failed

Jan  1 00:00:12 mrp: MRPD 012.013582 MSRP Listener (state = 1) event New!, VO->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.014913 MSRP Listener (state = 1) event New!, VO->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.016261 MSRP Listener (state = 1) event New!, VO->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.039990 MSRP Listener (state = 1) event New!, VO->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.043237 MSRP Listener (state = 1) event New!, VO->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.113639 MSRP Listener (state = 1) event tx!, VN->AN/MT
Jan  1 00:00:12 mrp: MRPD 012.113737 MSRP Listener (state = 1) event tx!, VN->AN/MT
Jan  1 00:00:12 mrp: MRPD 012.113766 MSRP Listener (state = 1) event tx!, VN->AN/MT
Jan  1 00:00:12 mrp: MRPD 012.113790 MSRP Listener (state = 1) event tx!, VN->AN/MT
Jan  1 00:00:12 mrp: MRPD 012.113814 MSRP Listener (state = 1) event tx!, VN->AN/MT
Jan  1 00:00:12 mrp: MRPD 012.113944 MSRP send PDU

Listener declares Listener Ready

Jan  1 00:00:12 mrp: MRPD 012.213363 MSRP Listener (state = 2) event New!, AN/MT
Jan  1 00:00:12 mrp: MRPD 012.214187 MSRP Listener (state = 2) event tx!, AN->AA/MT
Jan  1 00:00:12 mrp: MRPD 012.214222 MSRP Listener (state = 1) event tx!, AN->AA/MT
Jan  1 00:00:12 mrp: MRPD 012.214247 MSRP Listener (state = 1) event tx!, AN->AA/MT
Jan  1 00:00:12 mrp: MRPD 012.214270 MSRP Listener (state = 1) event tx!, AN->AA/MT
Jan  1 00:00:12 mrp: MRPD 012.214293 MSRP Listener (state = 1) event tx!, AN->AA/MT
Jan  1 00:00:12 mrp: MRPD 012.214445 MSRP send PDU
Jan  1 00:00:12 mrp: MRPD 012.215320 MSRP Listener (state = 2) event New!, AA->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.216787 MSRP Listener (state = 2) event New!, AA->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.217973 MSRP Listener (state = 2) event New!, AA->VN/MT
Jan  1 00:00:12 mrp: MRPD 012.219565 MSRP Listener (state = 2) event New!, AA->VN/MT

Switch sends rIn!

Jan  1 00:00:12 mrp: MRPD 012.308560 MSRP msrp_recv_msg() LISTENER vector addr 0x3, n = 5, VectorHeader  5
Jan  1 00:00:12 mrp: MRPD 012.308607 MSRP msrp_recv_msg() Listener[8][0] attrib 2
Jan  1 00:00:12 mrp: MRPD 012.309481 MSRP Listener (state = 2) event rIn!, AA->QA/MT
Jan  1 00:00:12 mrp: MRPD 012.309519 MSRP msrp_recv_msg() Listener[8][1] attrib 2
Jan  1 00:00:12 mrp: MRPD 012.309557 MSRP Listener (state = 1) event rIn!, VN/MT
Jan  1 00:00:12 mrp: MRPD 012.309583 MSRP msrp_recv_msg() Listener[8][2] attrib 2
Jan  1 00:00:12 mrp: MRPD 012.309616 MSRP Listener (state = 1) event rIn!, VN/MT
Jan  1 00:00:12 mrp: MRPD 012.309642 MSRP msrp_recv_msg() Listener[9][0] attrib 2
Jan  1 00:00:12 mrp: MRPD 012.309675 MSRP Listener (state = 1) event rIn!, VN/MT
Jan  1 00:00:12 mrp: MRPD 012.309700 MSRP msrp_recv_msg() Listener[9][1] attrib 2

[intel-ethernet]

Andrew –

I don’t understand the issue - you want the switch to only send Listener Asking Failed upstream to the talker and not reflected downstream back to the listener? Or you want the listener node to ignore listener attributes from the switch?

ekm

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 5:29 AM To: AVnu/Open-AVB Open-AVB@noreply.github.com Subject: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

The following sequence has been observed

1. Listener end station client declares Listener Asking Failed (because Talker declaration has not yet been observed)
2. Listener end station observes Talker declaration and then declares Listener Ready. Listener's mrpd waits for a transmit opportunity
3. Switch sends rIn! to end station declaring Listener Asking Failed. Merge operation in https://github.com/AVnu/Open-AVB/blob/master/daemons/mrpd/msrp.c#L276 causes declaration to be changed from the desired Listener Ready to Listener Asking Failed.

Captured logs for 5 streams follow (state 1 is Listener Asking Failed and state 2 is Listener Ready)

Listener declares Listener Asking Failed

Jan 1 00:00:12 mrp: MRPD 012.013582 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.014913 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.016261 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.039990 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.043237 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.113639 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113737 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113766 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113790 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113814 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113944 MSRP send PDU

Listener declares Listener Ready

Jan 1 00:00:12 mrp: MRPD 012.213363 MSRP Listener (state = 2) event New!, AN/MT

Jan 1 00:00:12 mrp: MRPD 012.214187 MSRP Listener (state = 2) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214222 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214247 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214270 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214293 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214445 MSRP send PDU

Jan 1 00:00:12 mrp: MRPD 012.215320 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.216787 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.217973 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.219565 MSRP Listener (state = 2) event New!, AA->VN/MT

Switch sends rIn!

Jan 1 00:00:12 mrp: MRPD 012.308560 MSRP msrp_recv_msg() LISTENER vector addr 0x3, n = 5, VectorHeader 5

Jan 1 00:00:12 mrp: MRPD 012.308607 MSRP msrp_recv_msg() Listener[8][0] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309481 MSRP Listener (state = 2) event rIn!, AA->QA/MT

Jan 1 00:00:12 mrp: MRPD 012.309519 MSRP msrp_recv_msg() Listener[8][1] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309557 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309583 MSRP msrp_recv_msg() Listener[8][2] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309616 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309642 MSRP msrp_recv_msg() Listener[9][0] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309675 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309700 MSRP msrp_recv_msg() Listener[9][1] attrib 2

— Reply to this email directly or view it on GitHubhttps://github.com/AVnu/Open-AVB/issues/338.

[andrew-elder]

Hi Eric,

Thanks for the feedback.

The problem is that within a listener end station, after the mrpd client sends a Listener Ready to mrpd, the Listener Ready declaration is "lost" within mrpd (it gets switched back to a Listener Asking Failed when a PDU is received from the switch). End result is that the talker never sees the Listener Ready.

Hope that makes sense.

I think the correct solution is to have the listener ignore the type/state of the listener attribute declared by the switch. After all, the Listener attribute declarations flow in a Listener -> switch(s) -> Talker direction.

I'll have a pull request in place soon for you to look at if you like. In terms of code changes it looks like

         /*
          * Listener attributes declared locally have
          * attrib->direction set to MSRP_DIRECTION_LISTENER.
          *
          * Listener attributes declared externally have
          * attrib->direction set to MSRP_DIRECTION_TALKER.
          *
          * Only support merging the substate when the directions match.
          */
         if (rattrib->direction == attrib->direction) {
             attrib->substate = rattrib->substate;
         }

in msrp_merge().

• Andrew

On 1/8/2016 11:34 AM, intel-ethernet wrote:

Andrew –

I don’t understand the issue - you want the switch to only send Listener Asking Failed upstream to the talker and not reflected downstream back to the listener? Or you want the listener node to ignore listener attributes from the switch?

ekm

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 5:29 AM To: AVnu/Open-AVB Open-AVB@noreply.github.com Subject: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

The following sequence has been observed

1. Listener end station client declares Listener Asking Failed (because Talker declaration has not yet been observed)
2. Listener end station observes Talker declaration and then declares Listener Ready. Listener's mrpd waits for a transmit opportunity
3. Switch sends rIn! to end station declaring Listener Asking Failed. Merge operation in https://github.com/AVnu/Open-AVB/blob/master/daemons/mrpd/msrp.c#L276 causes declaration to be changed from the desired Listener Ready to Listener Asking Failed.

Captured logs for 5 streams follow (state 1 is Listener Asking Failed and state 2 is Listener Ready)

Listener declares Listener Asking Failed

Jan 1 00:00:12 mrp: MRPD 012.013582 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.014913 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.016261 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.039990 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.043237 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.113639 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113737 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113766 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113790 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113814 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113944 MSRP send PDU

Listener declares Listener Ready

Jan 1 00:00:12 mrp: MRPD 012.213363 MSRP Listener (state = 2) event New!, AN/MT

Jan 1 00:00:12 mrp: MRPD 012.214187 MSRP Listener (state = 2) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214222 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214247 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214270 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214293 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214445 MSRP send PDU

Jan 1 00:00:12 mrp: MRPD 012.215320 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.216787 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.217973 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.219565 MSRP Listener (state = 2) event New!, AA->VN/MT

Switch sends rIn!

Jan 1 00:00:12 mrp: MRPD 012.308560 MSRP msrp_recv_msg() LISTENER vector addr 0x3, n = 5, VectorHeader 5

Jan 1 00:00:12 mrp: MRPD 012.308607 MSRP msrp_recv_msg() Listener[8][0] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309481 MSRP Listener (state = 2) event rIn!, AA->QA/MT

Jan 1 00:00:12 mrp: MRPD 012.309519 MSRP msrp_recv_msg() Listener[8][1] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309557 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309583 MSRP msrp_recv_msg() Listener[8][2] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309616 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309642 MSRP msrp_recv_msg() Listener[9][0] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309675 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309700 MSRP msrp_recv_msg() Listener[9][1] attrib 2

— Reply to this email directly or view it on GitHubhttps://github.com/AVnu/Open-AVB/issues/338.

— Reply to this email directly or view it on GitHub https://github.com/AVnu/Open-AVB/issues/338#issuecomment-170049123.

Andrew Elder AudioScience, Inc. +1-585-271-8870 www.audioscience.com

[intel-ethernet]

Can a talker also be a listener?

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 8:58 AM To: AVnu/Open-AVB Open-AVB@noreply.github.com Cc: Mann, Eric eric.mann@intel.com Subject: Re: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

Hi Eric,

Thanks for the feedback.

The problem is that within a listener end station, after the mrpd client sends a Listener Ready to mrpd, the Listener Ready declaration is "lost" within mrpd (it gets switched back to a Listener Asking Failed when a PDU is received from the switch). End result is that the talker never sees the Listener Ready.

Hope that makes sense.

I think the correct solution is to have the listener ignore the type/state of the listener attribute declared by the switch. After all, the Listener attribute declarations flow in a Listener -> switch(s) -> Talker direction.

I'll have a pull request in place soon for you to look at if you like. In terms of code changes it looks like

/*

• Listener attributes declared locally have
• attrib->direction set to MSRP_DIRECTION_LISTENER. *
• Listener attributes declared externally have
• attrib->direction set to MSRP_DIRECTION_TALKER. *
• Only support merging the substate when the directions match. */ if (rattrib->direction == attrib->direction) { attrib->substate = rattrib->substate; }

in msrp_merge().

• Andrew

On 1/8/2016 11:34 AM, intel-ethernet wrote:

Andrew –

I don’t understand the issue - you want the switch to only send Listener Asking Failed upstream to the talker and not reflected downstream back to the listener? Or you want the listener node to ignore listener attributes from the switch?

ekm

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 5:29 AM To: AVnu/Open-AVB Open-AVB@noreply.github.com<mailto:Open-AVB@noreply.github.com> Subject: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

The following sequence has been observed

1. Listener end station client declares Listener Asking Failed (because Talker declaration has not yet been observed)
2. Listener end station observes Talker declaration and then declares Listener Ready. Listener's mrpd waits for a transmit opportunity
3. Switch sends rIn! to end station declaring Listener Asking Failed. Merge operation in https://github.com/AVnu/Open-AVB/blob/master/daemons/mrpd/msrp.c#L276 causes declaration to be changed from the desired Listener Ready to Listener Asking Failed.

Captured logs for 5 streams follow (state 1 is Listener Asking Failed and state 2 is Listener Ready)

Listener declares Listener Asking Failed

Jan 1 00:00:12 mrp: MRPD 012.013582 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.014913 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.016261 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.039990 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.043237 MSRP Listener (state = 1) event New!, VO->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.113639 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113737 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113766 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113790 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113814 MSRP Listener (state = 1) event tx!, VN->AN/MT

Jan 1 00:00:12 mrp: MRPD 012.113944 MSRP send PDU

Listener declares Listener Ready

Jan 1 00:00:12 mrp: MRPD 012.213363 MSRP Listener (state = 2) event New!, AN/MT

Jan 1 00:00:12 mrp: MRPD 012.214187 MSRP Listener (state = 2) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214222 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214247 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214270 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214293 MSRP Listener (state = 1) event tx!, AN->AA/MT

Jan 1 00:00:12 mrp: MRPD 012.214445 MSRP send PDU

Jan 1 00:00:12 mrp: MRPD 012.215320 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.216787 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.217973 MSRP Listener (state = 2) event New!, AA->VN/MT

Jan 1 00:00:12 mrp: MRPD 012.219565 MSRP Listener (state = 2) event New!, AA->VN/MT

Switch sends rIn!

Jan 1 00:00:12 mrp: MRPD 012.308560 MSRP msrp_recv_msg() LISTENER vector addr 0x3, n = 5, VectorHeader 5

Jan 1 00:00:12 mrp: MRPD 012.308607 MSRP msrp_recv_msg() Listener[8][0] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309481 MSRP Listener (state = 2) event rIn!, AA->QA/MT

Jan 1 00:00:12 mrp: MRPD 012.309519 MSRP msrp_recv_msg() Listener[8][1] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309557 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309583 MSRP msrp_recv_msg() Listener[8][2] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309616 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309642 MSRP msrp_recv_msg() Listener[9][0] attrib 2

Jan 1 00:00:12 mrp: MRPD 012.309675 MSRP Listener (state = 1) event rIn!, VN/MT

Jan 1 00:00:12 mrp: MRPD 012.309700 MSRP msrp_recv_msg() Listener[9][1] attrib 2

— Reply to this email directly or view it on GitHubhttps://github.com/AVnu/Open-AVB/issues/338.

— Reply to this email directly or view it on GitHub https://github.com/AVnu/Open-AVB/issues/338#issuecomment-170049123.

Andrew Elder AudioScience, Inc. +1-585-271-8870 www.audioscience.comhttp://www.audioscience.com

— Reply to this email directly or view it on GitHubhttps://github.com/AVnu/Open-AVB/issues/338#issuecomment-170054942.

[andrew-elder]

A talker will never go "S+L:xxxx" to declare the stream it wishes to listen to. A talker will observe listener attributes that come over the network though. That is how it knows when to start streaming.

I may well have missed what you are getting at here.

The whole attribute direction thing is per stream/declaration of course....

On 1/8/2016 12:06 PM, intel-ethernet wrote:

Can a talker also be a listener?

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 8:58 AM To: AVnu/Open-AVB Open-AVB@noreply.github.com Cc: Mann, Eric eric.mann@intel.com Subject: Re: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

Hi Eric,

Thanks for the feedback.

The problem is that within a listener end station, after the mrpd client sends a Listener Ready to mrpd, the Listener Ready declaration is "lost" within mrpd (it gets switched back to a Listener Asking Failed when a PDU is received from the switch). End result is that the talker never sees the Listener Ready.

Hope that makes sense.

I think the correct solution is to have the listener ignore the type/state of the listener attribute declared by the switch. After all, the Listener attribute declarations flow in a Listener -> switch(s) -> Talker direction.

I'll have a pull request in place soon for you to look at if you like. In terms of code changes it looks like

/*

• Listener attributes declared locally have
• attrib->direction set to MSRP_DIRECTION_LISTENER. *
• Listener attributes declared externally have
• attrib->direction set to MSRP_DIRECTION_TALKER. *
• Only support merging the substate when the directions match. */ if (rattrib->direction == attrib->direction) { attrib->substate = rattrib->substate; }

in msrp_merge().

• Andrew

[pinealservo]

When an endpoint declaring a listener attribute gets an In state notification from its peer pertaining to that attribute, that just means that its link peer has registered our declaration. This is one of those bits of info that's mostly related to the other protocols built on MRP so that passive participants in a group don't have to declare attributes to take advantage of a group's existence. We definitely shouldn't be merging that attribute information into what we're declaring in MSRP; the merge function should probably only be called when we receive a Join-type state notification.

[andrew-elder]

I looked in 802.1Q for merging rules, but had trouble finding anything useful. Section 35.2.4.4 talks about Listener attribute propagation and 35.2.4.4.3 discusses merging of listener declarations, but some concepts seem to be missing.

[intel-ethernet]

Thanks for the clarification – I wasn’t clear whether you meant for a global mode which declared MRPD should operate in ‘talker mode’ or ‘listener mode’. You are proposing is an additional parameter to the attribute declaration to specify this direction (correct?).

ekm

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 9:16 AM To: AVnu/Open-AVB Cc: Mann, Eric Subject: Re: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

A talker will never go "S+L:xxxx" to declare the stream it wishes to listen to. A talker will observe listener attributes that come over the network though. That is how it knows when to start streaming.

I may well have missed what you are getting at here.

The whole attribute direction thing is per stream/declaration of course....

On 1/8/2016 12:06 PM, intel-ethernet wrote:

Can a talker also be a listener?

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 8:58 AM To: AVnu/Open-AVB Open-AVB@noreply.github.com<mailto:Open-AVB@noreply.github.com> Cc: Mann, Eric eric.mann@intel.com<mailto:eric.mann@intel.com> Subject: Re: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

Hi Eric,

Thanks for the feedback.

The problem is that within a listener end station, after the mrpd client sends a Listener Ready to mrpd, the Listener Ready declaration is "lost" within mrpd (it gets switched back to a Listener Asking Failed when a PDU is received from the switch). End result is that the talker never sees the Listener Ready.

Hope that makes sense.

I think the correct solution is to have the listener ignore the type/state of the listener attribute declared by the switch. After all, the Listener attribute declarations flow in a Listener -> switch(s) -> Talker direction.

I'll have a pull request in place soon for you to look at if you like. In terms of code changes it looks like

/*

• Listener attributes declared locally have
• attrib->direction set to MSRP_DIRECTION_LISTENER. *
• Listener attributes declared externally have
• attrib->direction set to MSRP_DIRECTION_TALKER. *
• Only support merging the substate when the directions match. */ if (rattrib->direction == attrib->direction) { attrib->substate = rattrib->substate; }

in msrp_merge().

• Andrew

— Reply to this email directly or view it on GitHubhttps://github.com/AVnu/Open-AVB/issues/338#issuecomment-170061877.

[intel-ethernet]

I’m not sure this fixes the issue – if multiple listeners declare the Listen attribute on the same stream ID, then wouldn’t the declarations from other listeners appear as Joins to peers?

ekm

From: Levi Pearson [mailto:notifications@github.com] Sent: Friday, January 8, 2016 10:11 AM To: AVnu/Open-AVB Cc: Mann, Eric Subject: Re: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

When an endpoint declaring a listener attribute gets an In state notification from its peer pertaining to that attribute, that just means that its link peer has registered our declaration. This is one of those bits of info that's mostly related to the other protocols built on MRP so that passive participants in a group don't have to declare attributes to take advantage of a group's existence. We definitely shouldn't be merging that attribute information into what we're declaring in MSRP; the merge function should probably only be called when we receive a Join-type state notification.

— Reply to this email directly or view it on GitHubhttps://github.com/AVnu/Open-AVB/issues/338#issuecomment-170077753.

[pinealservo]

There isn't any concept of merging at all in MRP, and the only concept of merging in MSRP is in listener attributes, which are solely part of MAP and thus aren't normally applicable to endpoints.

The merge function in the implementation source is not really doing merges in the sense of anything in the spec, or at least there's not a direct correspondence. It's more of a 'merge new information into our database' thing.

[pinealservo]

The MAP rules in bridge implementations won't declare listener attributes on any ports other than the one with a matching talker advertise attribute declaration, so other listeners don't generally know about each other except in a shared media situation.

Really, 'direction' shouldn't be talker vs. listener, it should be declare vs. register. We don't want to update any of our declarations based on hearing that others have registered them. MSRP state changes only happen based on declarations of link peers, i.e. we only act when we receive Join-type events rather than In-type ones.

[andrew-elder]

It turns our the "direction" parameter is already there and it is correctly filled out for all listener attributes. All I am proposing is that for listener attributes we inspect the direction parameter when deciding whether or not to merge attributes. It ends up being a one line change.

Now that I think I have fixed the listener side, I guess I need to write test cases for the talker side. I suspect the same thing could happen when transitioning from Talker Advertise to Talker Failed, but I have not checked that out yet.

• Andrew

On 1/8/2016 2:05 PM, intel-ethernet wrote:

Thanks for the clarification – I wasn’t clear whether you meant for a global mode which declared MRPD should operate in ‘talker mode’ or ‘listener mode’. You are proposing is an additional parameter to the attribute declaration to specify this direction (correct?).

ekm

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 9:16 AM To: AVnu/Open-AVB Cc: Mann, Eric Subject: Re: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

A talker will never go "S+L:xxxx" to declare the stream it wishes to listen to. A talker will observe listener attributes that come over the network though. That is how it knows when to start streaming.

I may well have missed what you are getting at here.

The whole attribute direction thing is per stream/declaration of course....

On 1/8/2016 12:06 PM, intel-ethernet wrote:

Can a talker also be a listener?

From: andrew-elder [mailto:notifications@github.com] Sent: Friday, January 8, 2016 8:58 AM To: AVnu/Open-AVB Open-AVB@noreply.github.com<mailto:Open-AVB@noreply.github.com> Cc: Mann, Eric eric.mann@intel.com<mailto:eric.mann@intel.com> Subject: Re: [Open-AVB] MRP: listener merge call can cause incorrect listener declaration (#338)

Hi Eric,

Thanks for the feedback.

The problem is that within a listener end station, after the mrpd client sends a Listener Ready to mrpd, the Listener Ready declaration is "lost" within mrpd (it gets switched back to a Listener Asking Failed when a PDU is received from the switch). End result is that the talker never sees the Listener Ready.

Hope that makes sense.

I think the correct solution is to have the listener ignore the type/state of the listener attribute declared by the switch. After all, the Listener attribute declarations flow in a Listener -> switch(s) -> Talker direction.

I'll have a pull request in place soon for you to look at if you like. In terms of code changes it looks like

/*

• Listener attributes declared locally have
• attrib->direction set to MSRP_DIRECTION_LISTENER. *
• Listener attributes declared externally have
• attrib->direction set to MSRP_DIRECTION_TALKER. *
• Only support merging the substate when the directions match. */ if (rattrib->direction == attrib->direction) { attrib->substate = rattrib->substate; }

in msrp_merge().

• Andrew

— Reply to this email directly or view it on GitHubhttps://github.com/AVnu/Open-AVB/issues/338#issuecomment-170061877.

— Reply to this email directly or view it on GitHub https://github.com/AVnu/Open-AVB/issues/338#issuecomment-170094422.

Andrew Elder AudioScience, Inc. +1-585-271-8870 www.audioscience.com

[andrew-elder]

Agree with 'direction' being replaced with declare and register. I'll look in to that when reviewing Talker attribute issues. It will make the code more obvious as well.

[pinealservo]

I suspect the same thing as well, Andrew. This is why 'updating' attributes rather than de-registering and re-registering is tricky; MRP didn't originally have any concept of attribute updates, as they were added for MSRP. The MRP state diagrams and specification text don't talk about this at all, it's just a couple of paragraphs in 35.2.6.

It seems, though, the direction field you're accessing now really is defined as talker vs. listener in 35.2.1.2. It is apparently meant to distinguish the various talker attributes from the listener attribute. This won't help you distinguish registrations from declarations, if that's the case.

[andrew-elder]

Ah, so it would seem that the direction field is incorrectly implemented at present and it is just by luck that I used to to solve the issue at hand. Currently any attributes created by a received PDU have the direction field set to 0. I need to add an action field that supports values of declare and register.

[eric-mann]

does this change apply to only listener attributes, or also to talker attributes?

I agree some method to differentiate locally declared vs registered attributes makes sense. I think the 'direction' field is mis-leading. I can't really remember that the 'direction' field was really ever used for anything - so perhaps rename the field altogether ('local'? 'declared'?).

fwiw - I know that some switch solution providers use the open-avb mrpd implementation as a baseline - so my other input is to resist putting in endpoint-only behaviors into the core MRP/M*RP routines ...

[andrew-elder]

Change will apply to listener and talker attributes. I've only created test case for listener attributes so far. And I have only observed the issue with listener attributes, but I'm 100% sure talker attributes have a similar issue as the code pathways are close to identical.

The OpenAVB MRP codebase was not using the direction field for anything until I decided to use it to solve this listener attribute merging issue. Levi provided references to where it is defined in 802.1Q, but because the direction values of LISTENER and TALKER are just labels, none of the MRP state machines made any reference to the direction setting at all. Furthermore, 802.1Q does not elaborate on exactly what to do with the direction field. I would be in favor of removing it completely as it is essentially unused code within MRP. The field would then be replaced with something else TBD to indicate whether attribute is declared or registered.

The changes to merging operations will have zero impact on switch implementations. I used endstation in my description of the issue because that is how I first observed the issue. A switch would observe and register an attribute on one port and then declare and emit the same attribute on another port, so the same pattern of propagation that I observed between an endstation and a switch would occur between two switches. That's my understanding at least. Levi might have more comments on this.

[eric-mann]

While on the topic of merging attributes ... I remember I had similar questions about other merge operations. You can see them by looking for the #defines ENABLEMERGED... - e.g. merging the latency field on talker attributes, as well as the priority on domains.

The latter seems just wrong as I believe this is how you would determine AVB network boundaries by mis-matches. I don't understand how the latency field is used in practice to have an opinion about the former.

[andrew-elder]

I've never built with either of ENABLE_MERGED_LATENCY or ENABLE_MERGED_PRIORITY defined.

I think the idea behind the latency is that the switch adds in the gPTP neighbor delay for port A when declaring the TA from port A on port B. That way the final listener knows the exact delay from all the links for the TA of interest. The TA latency is never actually merged because there would in independent instances of mrpd running for each port and an application would be response for forwarding declarations from one port to another. At least that is how I think about it, but I am by no means an expert on the subject.

On 1/9/2016 4:08 PM, Eric Mann wrote:

While on the topic of merging attributes ... I remember I had similar questions about other merge operations. You can see them by looking for the #defines ENABLEMERGED... - e.g. merging the latency field on talker attributes, as well as the priority on domains.

The latter seems just wrong as I believe this is how you would determine AVB network boundaries by mis-matches. I don't understand how the latency field is used in practice to have an opinion about the former.

— Reply to this email directly or view it on GitHub https://github.com/AVnu/Open-AVB/issues/338#issuecomment-170281128.

Andrew Elder AudioScience, Inc. +1-585-271-8870 www.audioscience.com

[andrew-elder]

I have now corrected Talker Advertise and Talker Failed merging. There are 3 new commits to the last pull request I submitted.

I have removed the "direction" field and replaced it with "operation" that supports values of DECLARE or REGISTER since that is what matches 802.1Q terminology.

I have not modified the DOMAIN processing at all as I'm not sure of the correct way to do it. The MRP application needs to know if the switch is sending a mismatched domain and then take action on that, but if I change the code so that the mismatch is never observed, the MRP application will never be notified with the mismatched values.

[eric-mann]

Looks good to me.

[pinealservo]

Just to be clear, the only 'merge' operation actually defined by the specification is only applicable to a multi-port bridge implementation, and it only applies to Listener attributes as they propagate (via the MAP program for MSRP) from the port at which they are registered to the port with a matching Talker Advertise registration. And its only purpose is to merge multiple listener attribute registrations (when there are multiple ports on which there are listener attributes with the same stream ID) into a single listener declaration.

According to the underlying MRP state machines, there is no such thing as attribute merge or update. An attribute is an atomic, immutable unit; the only thing that can be done with it is to declare or withdraw it to one's link peer. What happens in MSRP is that a registration of an attribute on one port will sometimes cause declarations of a slightly different attribute on other ports as part of the attribute propagation process. And sometimes you can shortcut the process of withdrawing a declaration and making a new declaration that's slightly different, but this is always intended to work as if you had fully withdrawn the old attribute and then declared a new one.

I'm not entirely sure on the reasoning behind the 'merge' operation in the code right now, as it's been a long time since I read it in context. But I think it's probably attempting to simplify the protocol operation in a way that's not directly described in the specification. This may be fine, or it may be based on a misunderstanding of the specification and have some more lurking bugs. Unfortunately, I don't have time to look into it more closely now.

[andrew-elder]

@pinealservo - thanks for the elaboration. Current merge code could perhaps more correctly be called attribute update. When a Listener attribute is already registered in the MSRP database as an Asking Failed and a PDU is observed with the same Listener attribute declared as Ready, the entry in the database is updated to be Ready. A similar operation occurs for Talker Advertise and Talker Failed.

You are correct in the sense that because the MRP daemon connects to a single port it does not implement a "merge" operation in the manner described in 802.1Q for propagating attributes across multiple ports.

[andrew-elder]

This has been fixed.