From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Jander Subject: Re: j1939: bind() and connect() Date: Tue, 15 Aug 2017 12:24:53 +0200 Message-ID: <20170815122453.3945bead@erd980> References: <048c5290-6cd0-aaec-8a74-1f38cbeb9efe@pengutronix.de> <20170815110116.4d12f4f3@erd980> <7164d44c-76f9-45f6-b5fa-c931f058163f@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from protonic.xs4all.nl ([83.163.252.89]:24796 "EHLO protonic.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753490AbdHOKYz (ORCPT ); Tue, 15 Aug 2017 06:24:55 -0400 In-Reply-To: <7164d44c-76f9-45f6-b5fa-c931f058163f@pengutronix.de> Sender: linux-can-owner@vger.kernel.org List-ID: To: Marc Kleine-Budde Cc: Kurt Van Dijck , linux-can On Tue, 15 Aug 2017 11:16:44 +0200 Marc Kleine-Budde wrote: > On 08/15/2017 11:01 AM, David Jander wrote: > > On Tue, 15 Aug 2017 10:14:18 +0200 > > Marc Kleine-Budde wrote: > > > >> On 08/14/2017 07:37 PM, Marc Kleine-Budde wrote: > >>> - Should a second bind() be allowed on a previously bind() socket? > >>> > >>> As far as I understand the code, there's some support, but when the > >>> interface is switched, j1939_ifindex_stop() is not called. > >> > >> Ah, there's a check, that you cannot re-bind() to another interface: > >> > >>> if (bound_dev_if != addr->can_ifindex) > > > > This makes sense. re-bind() to another interface only would make sense if > > bind() to any interface is also permitted, but that would imply separate > > address-claims on all different interfaces with potentially different > > source-address being claimed on each one.... don't think the stack should take > > care of such complex scenarios, at least not at first IMHO. > > Bind to any interface doesn't work, you cannot send anything: > > > static int j1939sk_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) > > { > [...] > > ifindex = jsk->ifindex_started; > > if (!ifindex) > > return -EBADFD; > > And you will probably not receive anything, as the rx-register, which > takes place in j1939_ifindex_start() is not done if you try to bind to > any device (bound_dev_if == 0). > > > if (bound_dev_if && bound_dev_if != jsk->ifindex_started) { > [...] > > > > ret = j1939_ifindex_start(bound_dev_if); > > Then I'll add proper test, that you have to bind to a specific > interface. That will make the code much simpler. I agree. Don't know if the case for bind-any will ever be necessary in the context of J1939, but we certainly should keep things simple for now. I've had cases myself where an ECU would talk J1939 on two interfaces at the same time, but not yet something like a Group-Function server exporting the same functions on more than one interface. I could think of that use-case happening though, but it can also be solved easily in user-space opening more than one socket... Best regards, -- David Jander Protonic Holland.