Bug 1148 - SlimServer can cause problems with network firmware devices (for example packet8 VoIP service) because of UDP arp packets to disconnected slimp3 client
: SlimServer can cause problems with network firmware devices (for example pack...
Status: RESOLVED DUPLICATE of bug 1147
Product: Logitech Media Server
Classification: Unclassified
Component: Streaming From SlimServer
: 5.x or older
: Other Linux (other)
: P3 major (vote)
: ---
Assigned To: Dan Sully
http://www.realisticsoftware.com/PrIv...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-03-21 14:24 UTC by Brian Beardmore
Modified: 2008-12-18 11:53 UTC (History)
0 users

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Beardmore 2005-03-21 14:24:09 UTC
Summary: SlimServer can cause problems with network firmware devices (for example packet8 VoIP 
service) because of UDP arp packets to disconnected slimp3 client

The SlimServer has exposed a defect in network firmware devices of lesser quality because they are 
unavailable to handle broadcast UDP arp packets correctly.  The SlimServer generates broadcast UDP 
arp packets when one of its clients are down, then these packets trigger failure on other devices.  The 
DTA310 used by the packet8 VoIP is one of these problem devices.  Ideally the network hardware 
firmware devices should fix their problem, but the slimserver software is easier to change.  I have a low 
impact software code fix that addresses the issue.

The packet8 service becomes unusable with the broadcast UDP arp packets (poor sound quality, 
incoming and outgoing voice breakups, complete phone call disconnects, phone calls dialed not 
completing).  I have also noticed that the UDP broadcast also caused minor problems for both of my 
cable-modem and netgear gateway because ever since I made the SlimServer code changes all of my 
third party network devices have had much fewer problems.  When the SlimServer UDP arp broadcasts 
were present I was resetting one or both devices several times a week.

I have tested my code fix with version 5.4.0 on a Qube3.  I have also quickly verified the existences of 
the bug and fix in versions 5.4.1, 6.0b1, and 6.0b2 on a Qube3.

I have identified two areas in the code that address this issue.  The first area is in the hello discovery 
code in the initialization.  The second area is in the general overall networking code that currently does 
not address a slim device vanishing from the network.  I have no way of knowing if these are issues for 
the squeezbox, but given that the squeezebox code appears to be a TCP/IP based connection; I believe 
this problem is exclusive to the slimp3.  My proposed changes are mainly for the slimp3 and should not 
change the current behavior of the squeezebox.

I have not had a great deal of time to devote to understanding the slimserver code, but I have 
addressed the Hello discovery code (which continuously looped forever sending UDP arp packets to 
disconnected slimp3) by simply adding a timeout value to the networking class initialization.  I 
addressed the issue of the slimp3 vanishing from the network by adding a time stamp field to the 
player record and then updating the time stamping every time any network traffic from a player was 
received by the slimserver.  I also added an internal timer loop to monitor the timestamp and ping the 
device as necessary.  If the timestamp goes out of range the client is removed from the internal list.

The URL attached to this bug is a zip file with all of my code modification for this bug for the 5.4.0, 
5.4.1, and 6.0b1, 6.0b2 releases.  The file diffs are at the end of the bug report as well.  I hope you are 
able to use my codes change directly.

There is one minor existing or exposed issue that I have not looked into.  If the slimp3 device is in the 
off state and displaying the time when it is disconnected from the network, when the slimp3 comes 
back on it returns to the on state with what it was last playing music wise.  It appears that the off state 
is not saved or is not restored from the saved configuration information.  The on state of the player is 
restored without a problem when the device is disconnected from the network and reconnected at a 
later time.

Please contact me for any follow up, if it is needed.


Note: this bug occurs in SlimServer versions 5.4.0, 5.4.1, 6.0b1, and 6.0b2 

LAN hardware involved
---------------------
Qube3 (slimserver)
  - patched with all the latest updates
  - used perl 5.4.1 or 5.4.6 
Netgear 10/100 switch
Netgear WGT624 gateway
Zoom cable modem
DTA-310 (packet8 VoIP) device latest firmware v1034
Macs & PCs on and off network
slimp3 (powered down every night and other times when it is not being used)
Comcast ISP


Steps to reproduce
------------------
-Network connect slimp3 and qube3 and DTA-310 and gateway to network
-Power up gateway and setup
-Power up DTA310 and setup
-Power on slimp3
-Setup slimserver on Qube
-Use slimp3 and verify all the setup is working
-Log into Qube and watch UDP network port (tcpdump)
-Disconnect the slimp3 from network (wait about 30 seconds)
* See the continuous UDP arps and the DTA-310 link light now flashes


File Diffs v5.4.1
-----------------
#####Protocol.pm
28a29
> my $SLIMECHO_PORT = 7;                # Echo port on the Slim devices
80c81,82
<               LocalAddr => $main::localClientNetAddr
---
>               LocalAddr => $main::localClientNetAddr,
>               Timeout   => Slim::Utils::Prefs::get('remotestreamtimeout')
125,160c127,136
< 
<                       # check that it's a message type we know: starts with i r 2 d a or h (but not h followed 
by 0x00 0x00)
<                       if ($msg =~ /^(?:[ir2a]|h(?!\x00\x00))/) {
<                               my $client = getUdpClient($clientpaddr, $sock, $msg);
< 
<                               if (!defined($client)) {
<                                       return;
<                               }
< 
<                               if ($SIMULATE_RX_DELAY) {
<                                       # simulate rx delay
<                                       Slim::Utils::Timers::setTimer($client, Time::HiRes::time() + 
$SIMULATE_RX_DELAY/1000,
<                                                               \&Slim::Networking::Protocols::processMessage, $msg);
<                               } else {
<                                       processMessage($client, $msg);
<                               }
< 
<                       } elsif ($msg =~/^d/) {
<                               # Discovery request: note that slimp3 sends deviceid and revision in the discovery
<                               # request, but the revision is wrong (v 2.2 sends revision 1.1). Oops. 
<                               # also, it does not send the MAC address until the [h]ello packet.
<                               # Squeezebox sends all fields correctly.
< 
<                               my ($msgtype, $deviceid, $revision, @mac) = unpack 
'axCCxxxxxxxxH2H2H2H2H2H2', $msg;
<                               my $mac = join(':', @mac);
<                               Slim::Network::Discovery::gotDiscoveryRequest($sock, $clientpaddr, $deviceid, 
$revision, $mac);
< 
<                       # Playlist::executecommand can be accessed over the UDP port
<                       } elsif ($msg=~/^executecommand\((.*)\)$/) {
<                               my $ecArgs=$1;
<                               my @ecArgs=split(/, ?/, $ecArgs);
<                               $::d_protocol && msg("UDP: executecommand($ecArgs)\n");
<                               my $clientipport = shift(@ecArgs);
<                               my $client = Slim::Player::Client::getClient($clientipport);
<                               Slim::Control::Command::execute($client, \@ecArgs);
< 
---
>                       my ($clientport, $clientip) = sockaddr_in($clientpaddr);
> 
>                       if ($clientport eq Slim::Networking::Slimproto::getSlimProtoPort()) {
>                               # handle the udp packets that came from client the SLIMPROTO_PORT(3483)
>                               processReplyFromSlimPort($sock, $clientpaddr, $clientip, $msg);
> 
>                       } elsif ($clientport eq $SLIMECHO_PORT) {
>                               # handle the udp packets that came from client the echo port(7)
>                               processReplyFromEchoPort($sock, $clientpaddr, $msg);
> 
164d139
<                                       my ($clientport, $clientip) = sockaddr_in($clientpaddr);
170a146,233
> }
> 
> sub processReplyFromSlimPort {
>       my ($sock, $clientpaddr, $clientip, $msg) = @_;
> 
>       # check that it's a message type we know: starts with i r 2 d a or h (but not h followed by 
0x00 0x00)
>       if ($msg =~ /^(?:[ir2a]|h(?!\x00\x00))/) {
>               my $client = getUdpClient($clientpaddr, $sock, $msg);
> 
>               if (!defined($client)) {
>                       return;
>               }
> 
>               if ($SIMULATE_RX_DELAY) {
>                       # simulate rx delay
>                       Slim::Utils::Timers::setTimer($client, Time::HiRes::time() + $SIMULATE_RX_DELAY/
1000,
>                                               \&Slim::Networking::Protocols::processMessage, $msg);
>               } else {
>                       processMessage($client, $msg);
>               }
>               $client->lastReplyTime(Time::HiRes::time());
> 
>       } elsif ($msg =~/^d/) {
>               # Discovery request: note that slimp3 sends deviceid and revision in the discovery
>               # request, but the revision is wrong (v 2.2 sends revision 1.1). Oops. 
>               # also, it does not send the MAC address until the [h]ello packet.
>               # Squeezebox sends all fields correctly.
> 
>               my ($msgtype, $deviceid, $revision, @mac) = unpack 'axCCxxxxxxxxH2H2H2H2H2H2', 
$msg;
>               my $mac = join(':', @mac);
>               Slim::Network::Discovery::gotDiscoveryRequest($sock, $clientpaddr, $deviceid, $revision, 
$mac);
> 
>       # Playlist::executecommand can be accessed over the UDP port
>       } elsif ($msg=~/^executecommand\((.*)\)$/) {
>               my $ecArgs=$1;
>               my @ecArgs=split(/, ?/, $ecArgs);
>               $::d_protocol && msg("UDP: executecommand($ecArgs)\n");
>               my $clientipport = shift(@ecArgs);
>               my $client = Slim::Player::Client::getClient($clientipport);
>               Slim::Control::Command::execute($client, \@ecArgs);
>               $client->lastReplyTime(Time::HiRes::time());
> 
>       } else {
>               if ($::d_protocol) {
>                       msg("ignoring Client: ".inet_ntoa($clientip).":slimport that sent bogus message 
$msg\n");
>               }
>       }
> } 
> 
> sub processReplyFromEchoPort {
>       my ($sock, $clientpaddr, $msg) = @_;
>       my $client;
>       my @mac;
>       my $txt;
>       my $macaddr;
> 
>       ( $txt, $mac[0], $mac[1], $mac[2], $mac[3], $mac[4], $mac[5] ) 
>               = unpack ('a4H2H2H2H2H2H2', $msg);
>       $macaddr = join(':', @mac);
> 
>       $client = Slim::Player::Client::getClient($macaddr);
>       if (defined($client)) {
>               $client->lastReplyTime(Time::HiRes::time());
>               if ($::d_protocol) {
>                       my ($clientport, $clientip) = sockaddr_in($client->paddr);
>                       msg("processReplyFromEchoPort: recv text of '$txt'\n  from Client: "
>                               .inet_ntoa($clientip)." lastReplyTime was set\n");
>               }
>       } else {
>               $::d_protocol && msg("processReplyFromEchoPort: recv text of '$txt'\n");
>       }
> 
> }
> 
> sub sendEchoToClient {
>       my $client = shift;
> 
>       my @mac = split(':', $client->macaddress());
>       my $paddr = ipaddress2paddr($client->ip.":".$SLIMECHO_PORT);
>       my $frame = pack 'a4H2H2H2H2H2H2', (' Yo!', 
>                       $mac[0], $mac[1], $mac[2], $mac[3], $mac[4], $mac[5] );
> 
>       $udpsock->send( $frame, 0, $paddr);
> 
>       if ($::d_protocol) {
>               my ($clientport, $clientip) = sockaddr_in($client->paddr);
>               msg("sendEchoToClient sent to Client: ".inet_ntoa($clientip)."\n");
>       }
#####Slimproto.pm
522a523,526
> sub getSlimProtoPort {
>       return ($SLIMPROTO_PORT);
> }
> 
#####Client.pm
744a745
>       $client->[93] = undef; # lastReplyTime
881a883,896
> sub forgetlostClient {
>       my $id = shift;
>       my $client = getClient($id);
> 
>       if ($client) {
>               Slim::Web::HTTP::forgetClient($client);
>               #Slim::Player::Playlist::forgetClient($client);
>                       # trying to play will close out any open files.
>                       Slim::Control::Command::execute($client, ["play"]);
>               Slim::Utils::Timers::forgetClient($client);
>               delete $clientHash{$id};
>       }
> }
> 
1430a1446,1451
> 
> sub lastReplyTime {
>       my $r = shift;
>       @_ ? ($r->[93] = shift) : $r->[93];
> }
> 
#####SLIMP3.pm
17a18,24
> use Slim::Utils::Timers;
> use Slim::Player::Client;
> use Slim::Networking::Protocol;
> use Socket;           # for sockaddr_in
> 
> my $CLIENTTIMEOUT = 30;               # in seconds
> my $CHECKCLIENTEVERY = 5;             # in seconds
53c60,64
< 
---
> 
>       # setup timer to keep checking if client is still available
>       $client->lastReplyTime(Time::HiRes::time());
>       Slim::Utils::Timers::setTimer($client, (Time::HiRes::time() + $CHECKCLIENTEVERY), 
\&checkSLIMP3);
> 
56a68,94
> sub checkSLIMP3 {
>       my $client = shift;
>       my $now;
>       my $deltaTime;
> 
>       $now = Time::HiRes::time();
>       $deltaTime = $now-$client->lastReplyTime;
> 
>       if ($deltaTime > $CLIENTTIMEOUT ) {
>               # we have lost the client, so clean up
>               my ($clientport, $clientip) = sockaddr_in($client->paddr);
>               msg("Client: ".inet_ntoa($clientip)." last responded $deltaTime seconds ago and is being 
timed out.\n");
>               msg("The client is being forgotten about\n");
>               #Slim::Player::Client::forgetClient($client->id());
>               Slim::Player::Client::forgetlostClient($client->id());
> 
>       } else {
>               if (($now-$client->lastReplyTime) > $CHECKCLIENTEVERY ) {
>                       # only send the echo packet if we have not heard from the client anyway
>                       Slim::Networking::Protocol::sendEchoToClient($client);
>               }
> 
>               # Call ourselves again after $CHECKCLIENTEVERY seconds
>               Slim::Utils::Timers::setTimer($client, ($now + $CHECKCLIENTEVERY), \&checkSLIMP3);
>       }
> }
> 
177c215,216
<       send($client->udpsock, $frame, 0, $client->paddr()); 
---
>       #send($client->udpsock, $frame, 0, $client->paddr());   # changed to fix timeout bug BAB 
2005.02.09
>       $client->udpsock->send($frame, 0, $client->paddr());
192c231,232
<       send($client->udpsock, $frame, 0, $client->paddr());
---
>       #send($client->udpsock, $frame, 0, $client->paddr());   # changed to fix timeout bug BAB 
2005.02.09
>       $client->udpsock->send($frame, 0, $client->paddr());
200c240,241
<       send($client->udpsock, '2                 '.$data, 0, $client->paddr);
---
>       #send($client->udpsock, '2                 '.$data, 0, $client->paddr);         # changed to fix timeout 
bug BAB 2005.02.09
>       $client->udpsock->send('2                 '.$data, 0, $client->paddr);
Comment 1 KDF 2005-03-21 14:54:35 UTC

*** This bug has been marked as a duplicate of 1147 ***
Comment 2 Chris Owens 2008-12-18 11:53:55 UTC
Routine bug db maintenance; removing old versions which cause confusion.  I apologize for the inconvenience.