netdev - Re: [PATCH V3 net-next] LISP: Locator/Identifier Separation Protocol

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <6A1822EF-41D2-49AC-AA94-227458773FF8@logicalelegance.com>
Date:	Thu, 19 Jun 2014 09:31:27 -0700
From:	Christopher White <chris@...icalelegance.com>
To:	Tom Herbert <therbert@...gle.com>
Cc:	Linux Netdev List <netdev@...r.kernel.org>,
	"Vina Ermagan (vermagan)" <vermagan@...co.com>,
	"Lorand Jakab -X (lojakab - M SQUARED CONSULTING INC. at Cisco)" 
	<lojakab@...co.com>
Subject: Re: [PATCH V3 net-next] LISP: Locator/Identifier Separation Protocol

Hi Tom, 

Thanks again for the quick turnaround. Some responses and questions below…

On Jun 18, 2014, at 5:08 PM, Tom Herbert <therbert@...gle.com> wrote:
>> 
>> +       hash = skb_get_hash(skb);
>> +       if (!hash)
>> +               hash = jhash(skb->data, 2 * ETH_ALEN,
>> +                            (__force u32) skb->protocol);
>> +
> 
> This is probably okay for now, but we should really be smarter here.
> Looks like another consumer of a TX hash in skbuf (Eric was looking to
> add that I believe).

Ok.

> >check = CSUM_MANGLED_0;
>> +               }
>> +
>> +               skb->protocol = protocol;
>> +       } while ((skb = skb->next));
>> +out:
>> +       return segs;
>> +}
>> +
> Please split out generic changes into their own patches.
> 

Will do.

>> +int udpv4_create_encap_sock(struct net *net, __be16 port, struct socket **psock,
>> +                           bool csum)
>> +{
>> +       struct sock *sk;
>> +       struct socket *sock;
>> +       struct sockaddr_in lisp_addr = {
>> +               .sin_family             = AF_INET,
>> +               .sin_addr.s_addr        = htonl(INADDR_ANY),
>> +               .sin_port               = port,
>> +       };
>> +       int rc;
>> +
> Still some lisp artifacts.

Sorry, that got past me, will fix.

>> 
>> +       }
>> +
>> +       *psock = sock;
>> +       /* Disable multicast loopback */
>> +       inet_sk(sk)->mc_loop = 0;
>> +
>> +       if (!csum)
>> +               sock->sk->sk_no_check_tx = 1;
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL(udpv4_create_encap_sock)
> 
> I was actually thinking this function could be even more general. The
> L2TP_ENCAPTYPE_UDP case in l2tp_tunnel_sock_create looks like
> something we might be able to abstract out into a separate function--
> it would include IPv6 support and the possibility of binding to other
> than INADDR_ANY and connected sockets. l2tp_tunnel_cfg could be the
> basis of udp_port_cfg which contains addresses, ports, and sockopts
> like sk_no_check_tx…
> 

So I’m happy to do this, but I would like to do it in a separate submission. 
For one thing, I am not super familiar with that code and I’m a bit nervous 
about making substantive changes, thus it may take me some time. 
As it is orthogonal to the LISP submission, would it be possible to do this 
separately and have LISP move forward?

Thanks!
-Chris

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html