miniupnp.tuxfamily.org Forum Index miniupnp.tuxfamily.org
The forum about miniupnp and libnatpmp
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

Service anouncement messages only being sent out once

 
Post new topic   Reply to topic    miniupnp.tuxfamily.org Forum Index -> miniupnpd Bugs
View previous topic :: View next topic  
Author Message
twear



Joined: 01 Nov 2012
Posts: 80

PostPosted: Fri Apr 19, 2013 12:20 am    Post subject: Service anouncement messages only being sent out once Reply with quote

So per the UPnP guidlines:

In UPnP Device Architecture 1.0, Section 1.1.2,
Due to the unreliable nature of UDP, devices should send each of the above discovery messages more than once, although to avoid network congestion discovery messages should not be sent more than three times.

I am however only see the messages being sent out once, after investigating the code I confirmed this. A simple fix was too add in the for loop which I got from MiniDLNA:

Code:
for (dup = 0; dup < 2; dup++)
   {
          <code>
        }



can we get these changes added to the mainline?
Back to top
View user's profile Send private message
twear



Joined: 01 Nov 2012
Posts: 80

PostPosted: Fri Apr 19, 2013 6:02 pm    Post subject: Reply with quote

This is the patch, version 1.8

Code:
diff --git a/minissdp.c b/minissdp.c
old mode 100644
new mode 100755
index f937109..bfe105b
--- a/minissdp.c
+++ b/minissdp.c
@@ -394,6 +394,16 @@ static struct {
 };
 
 static void
+_usleep(long usecs)
+{
+   struct timespec sleep_time;
+
+   sleep_time.tv_sec = 0;
+   sleep_time.tv_nsec = usecs * 1000;
+   nanosleep(&sleep_time, NULL);
+}
+
+static void
 SendSSDPNotifies(int s, const char * host, unsigned short port,
                  unsigned int lifetime, int ipv6)
 {
@@ -402,7 +412,7 @@ SendSSDPNotifies(int s, const char * host, unsigned short port,
 #else
    struct sockaddr_in sockname;
 #endif
-   int l, n, i=0;
+   int l, n, dup, i=0;
    char bufr[512];
    char ver_str[4];
 
@@ -424,58 +434,66 @@ SendSSDPNotifies(int s, const char * host, unsigned short port,
       p->sin_addr.s_addr = inet_addr(SSDP_MCAST_ADDR);
    }
 
-   while(known_service_types[i].s)
+
+   for( dup=0; dup<2; dup++ )
    {
-      if(i==0)
-         ver_str[0] = '\0';
-      else
-         snprintf(ver_str, sizeof(ver_str), "%d", known_service_types[i].version);
-      l = snprintf(bufr, sizeof(bufr),
-         "NOTIFY * HTTP/1.1\r\n"
-         "HOST: %s:%d\r\n"
-         "CACHE-CONTROL: max-age=%u\r\n"
-         "lOCATION: http://%s:%d" ROOTDESC_PATH"\r\n"
-         "SERVER: " MINIUPNPD_SERVER_STRING "\r\n"
-         "NT: %s%s\r\n"
-         "USN: %s::%s%s\r\n"
-         "NTS: ssdp:alive\r\n"
-         "OPT: \"http://schemas.upnp.org/upnp/1/0/\"; ns=01\r\n" /* UDA v1.1 */
-         "01-NLS: %u\r\n" /* same as BOOTID field. UDA v1.1 */
-         "BOOTID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
-         "CONFIGID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
-         "\r\n",
-         ipv6 ? "[" LL_SSDP_MCAST_ADDR "]" : SSDP_MCAST_ADDR,
-         SSDP_PORT,
-         lifetime,
-         host, port,
-         known_service_types[i].s, ver_str,
-         uuidvalue, known_service_types[i].s, ver_str,
-         upnp_bootid, upnp_bootid, upnp_configid );
-      if(l<0)
-      {
-         syslog(LOG_ERR, "SendSSDPNotifies() snprintf error");
-         continue;
-      }
-      if((unsigned int)l >= sizeof(bufr))
-      {
-         syslog(LOG_WARNING, "SendSSDPNotifies(): truncated output");
-         l = sizeof(bufr);
-      }
-      n = sendto(s, bufr, l, 0,
-         (struct sockaddr *)&sockname,
-#ifdef ENABLE_IPV6
-         ipv6 ? sizeof(struct sockaddr_in6) : sizeof(struct sockaddr_in)
-#else
-         sizeof(struct sockaddr_in)
-#endif
-         );
-      if(n < 0)
+
+      if( dup )
+         _usleep(200000);
+      i=0;
+      while(known_service_types[i].s)
       {
-         /* XXX handle EINTR, EAGAIN, EWOULDBLOCK */
-         syslog(LOG_ERR, "sendto(udp_notify=%d, %s): %m", s,
-                host ? host : "NULL");
+         if(i==0)
+            ver_str[0] = '\0';
+         else
+            snprintf(ver_str, sizeof(ver_str), "%d", known_service_types[i].version);
+         l = snprintf(bufr, sizeof(bufr),
+            "NOTIFY * HTTP/1.1\r\n"
+            "HOST: %s:%d\r\n"
+            "CACHE-CONTROL: max-age=%u\r\n"
+            "lOCATION: http://%s:%d" ROOTDESC_PATH"\r\n"
+            "SERVER: " MINIUPNPD_SERVER_STRING "\r\n"
+            "NT: %s%s\r\n"
+            "USN: %s::%s%s\r\n"
+            "NTS: ssdp:alive\r\n"
+            "OPT: \"http://schemas.upnp.org/upnp/1/0/\"; ns=01\r\n" /* UDA v1.1 */
+            "01-NLS: %u\r\n" /* same as BOOTID field. UDA v1.1 */
+            "BOOTID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
+            "CONFIGID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
+            "\r\n",
+            ipv6 ? "[" LL_SSDP_MCAST_ADDR "]" : SSDP_MCAST_ADDR,
+            SSDP_PORT,
+            lifetime,
+            host, port,
+            known_service_types[i].s, ver_str,
+            uuidvalue, known_service_types[i].s, ver_str,
+            upnp_bootid, upnp_bootid, upnp_configid );
+         if(l<0)
+         {
+            syslog(LOG_ERR, "SendSSDPNotifies() snprintf error");
+            continue;
+         }
+         if((unsigned int)l >= sizeof(bufr))
+         {
+            syslog(LOG_WARNING, "SendSSDPNotifies(): truncated output");
+            l = sizeof(bufr);
+         }
+         n = sendto(s, bufr, l, 0,
+            (struct sockaddr *)&sockname,
+   #ifdef ENABLE_IPV6
+            ipv6 ? sizeof(struct sockaddr_in6) : sizeof(struct sockaddr_in)
+   #else
+            sizeof(struct sockaddr_in)
+   #endif
+            );
+         if(n < 0)
+         {
+            /* XXX handle EINTR, EAGAIN, EWOULDBLOCK */
+            syslog(LOG_ERR, "sendto(udp_notify=%d, %s): %m", s,
+                  host ? host : "NULL");
+         }
+         i++;
       }
-      i++;
    }
 }
 
@@ -720,7 +738,7 @@ SendSSDPGoodbye(int * sockets, int n_sockets)
    int i, j;
    char bufr[512];
    char ver_str[4];
-   int ret = 0;
+   int dup, ret = 0;
    int ipv6 = 0;
 
     memset(&sockname, 0, sizeof(struct sockaddr_in));
@@ -734,49 +752,52 @@ SendSSDPGoodbye(int * sockets, int n_sockets)
    inet_pton(AF_INET6, LL_SSDP_MCAST_ADDR, &(sockname6.sin6_addr));
 #endif
 
-   for(j=0; j<n_sockets; j++)
+   for (dup = 0; dup < 2; dup++)
    {
-#ifdef ENABLE_IPV6
-      ipv6 = j & 1;
-#endif
-       for(i=0; known_service_types[i].s; i++)
-       {
-         if(i==0)
-            ver_str[0] = '\0';
-         else
-            snprintf(ver_str, sizeof(ver_str), "%d", known_service_types[i].version);
-           l = snprintf(bufr, sizeof(bufr),
-                 "NOTIFY * HTTP/1.1\r\n"
-                 "HOST: %s:%d\r\n"
-                 "NT: %s%s\r\n"
-                 "USN: %s::%s%s\r\n"
-                 "NTS: ssdp:byebye\r\n"
-             "OPT: \"http://schemas.upnp.org/upnp/1/0/\"; ns=01\r\n" /* UDA v1.1 */
-             "01-NLS: %u\r\n" /* same as BOOTID field. UDA v1.1 */
-             "BOOTID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
-             "CONFIGID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
-                 "\r\n",
-                 ipv6 ? "[" LL_SSDP_MCAST_ADDR "]" : SSDP_MCAST_ADDR,
-              SSDP_PORT,
-             known_service_types[i].s, ver_str,
-                 uuidvalue, known_service_types[i].s, ver_str,
-                 upnp_bootid, upnp_bootid, upnp_configid);
-           n = sendto(sockets[j], bufr, l, 0,
-#ifdef ENABLE_IPV6
-                      ipv6 ? (struct sockaddr *)&sockname6 : (struct sockaddr *)&sockname,
-                      ipv6 ? sizeof(struct sockaddr_in6) : sizeof(struct sockaddr_in)
-#else
-                      (struct sockaddr *)&sockname,
-                      sizeof(struct sockaddr_in)
-#endif
-                    );
-         if(n < 0)
+      for(j=0; j<n_sockets; j++)
+      {
+   #ifdef ENABLE_IPV6
+         ipv6 = j & 1;
+   #endif
+         for(i=0; known_service_types[i].s; i++)
          {
-            syslog(LOG_ERR, "SendSSDPGoodbye: sendto(udp_shutdown=%d): %m",
-                   sockets[j]);
-            ret = -1;
+            if(i==0)
+               ver_str[0] = '\0';
+            else
+               snprintf(ver_str, sizeof(ver_str), "%d", known_service_types[i].version);
+            l = snprintf(bufr, sizeof(bufr),
+                "NOTIFY * HTTP/1.1\r\n"
+                "HOST: %s:%d\r\n"
+                "NT: %s%s\r\n"
+                "USN: %s::%s%s\r\n"
+                "NTS: ssdp:byebye\r\n"
+                "OPT: \"http://schemas.upnp.org/upnp/1/0/\"; ns=01\r\n" /* UDA v1.1 */
+                "01-NLS: %u\r\n" /* same as BOOTID field. UDA v1.1 */
+                "BOOTID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
+                "CONFIGID.UPNP.ORG: %u\r\n" /* UDA v1.1 */
+                "\r\n",
+                ipv6 ? "[" LL_SSDP_MCAST_ADDR "]" : SSDP_MCAST_ADDR,
+                SSDP_PORT,
+                known_service_types[i].s, ver_str,
+                uuidvalue, known_service_types[i].s, ver_str,
+                upnp_bootid, upnp_bootid, upnp_configid);
+            n = sendto(sockets[j], bufr, l, 0,
+   #ifdef ENABLE_IPV6
+                     ipv6 ? (struct sockaddr *)&sockname6 : (struct sockaddr *)&sockname,
+                     ipv6 ? sizeof(struct sockaddr_in6) : sizeof(struct sockaddr_in)
+   #else
+                     (struct sockaddr *)&sockname,
+                     sizeof(struct sockaddr_in)
+   #endif
+                   );
+            if(n < 0)
+            {
+               syslog(LOG_ERR, "SendSSDPGoodbye: sendto(udp_shutdown=%d): %m",
+                     sockets[j]);
+               ret = -1;
+            }
          }
-       }
+      }
    }
    return ret;
 }
Back to top
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Mon Apr 22, 2013 1:10 pm    Post subject: Reply with quote

Your code is blocking...
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
twear



Joined: 01 Nov 2012
Posts: 80

PostPosted: Mon Apr 22, 2013 4:29 pm    Post subject: Reply with quote

how so?
Back to top
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Tue Apr 23, 2013 8:58 am    Post subject: Reply with quote

It is calling nanosleep().
To do things cleanly in the current architecture of miniupnpd, we should put the packets to send in a queue and send them from the main loop.
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
twear



Joined: 01 Nov 2012
Posts: 80

PostPosted: Tue Apr 23, 2013 11:44 pm    Post subject: Reply with quote

you can do it without the nanosleep, I just referenced the implementation from MiniDLNA
Back to top
View user's profile Send private message
twear



Joined: 01 Nov 2012
Posts: 80

PostPosted: Fri May 10, 2013 9:21 pm    Post subject: Reply with quote

is this something which is able to be fixed?
Back to top
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Sun May 12, 2013 9:39 pm    Post subject: Reply with quote

twear wrote:
is this something which is able to be fixed?

Of course it can be fixed, but it needs work. I have opened an Issue on the github : https://github.com/miniupnp/miniupnp/issues/35
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
twear



Joined: 01 Nov 2012
Posts: 80

PostPosted: Thu May 23, 2013 12:40 am    Post subject: Reply with quote

Can you update this thread when it is pushed to the mainline?

Thanks
Back to top
View user's profile Send private message
miniupnp
Site Admin


Joined: 14 Apr 2007
Posts: 1589

PostPosted: Tue Sep 15, 2015 7:49 am    Post subject: Reply with quote

twear wrote:
Can you update this thread when it is pushed to the mainline?

done Smile https://github.com/miniupnp/miniupnp/commit/9832adc45613d0b079ac8a09e49755ec01991736
_________________
Main miniUPnP author.
https://miniupnp.tuxfamily.org/
Back to top
View user's profile Send private message Visit poster's website
Display posts from previous:   
Post new topic   Reply to topic    miniupnp.tuxfamily.org Forum Index -> miniupnpd Bugs All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group
Protected by Anti-Spam ACP
© 2007 Thomas Bernard, author of MiniUPNP.