[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #3586: Revert a change that accepted UDP packets not destined for us.

[GitBox <gi...@apache.org>]

patacongo edited a comment on pull request #3586:
URL: https://github.com/apache/incubator-nuttx/pull/3586#issuecomment-824423075


   This is a strange change.  The title is not correct:  It is not accepting UDP packets that are not destined for us.  Something very different is going on.
   
   UDP packets are processed above (IPv6 is similar):
   
        switch (ipv4->proto)
           {
       ...
       #ifdef NET_UDP_HAVE_STACK
             case IP_PROTO_UDP:   /* UDP input */
               udp_ipv4_input(dev);
              break;
       #endif
   
   But in the default case of the switch, the packet is only dropped if
   
       if (ipv4->proto != IP_PROTO_UDP)
   
   Although it seems like the "dropped" statistics are incremented in any case.
   
   So bad UDP packets may slip through.  Normally, that default case will never be executed for a UDP packet.These "bad" UDP packets would be accepted if:
   
   1. UDP is not enabled.  THAT is a bug!!! If UDP is not enabled, then UDP packets much always be dropped.  That needs to be fixed.
   2. This can also happen is `NET_UDP_HAVE_STACK` is not defined.  That is, a USRSOCK UDP configuration is being used.  It is abysmal programming style that there are no comments explaining this in the code but this might be an acceptable case.  I would also like to hear the explanation.
   
   `NET_UDP_HAVE_STACK` is equivalent to:
   
       !defined(CONFIG_NET_UDP) || defined(CONFIG_NET_UDP_NO_STACK)
   
   It would be more correct if the `if `were conditioned on `#ifdef CONFIG_NET_UDP_NO_STACK`.  With that conditioning, I think the change is OK, isn't it?  Some additional explanation and inline comments are necessary.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org