On Thu, Jan 24, 2019 at 07:46:19PM +0100, Kurt Van Dijck wrote:
> On do, 24 jan 2019 09:13:29 +0100, Oleksij Rempel wrote:
> > On 24.01.19 08:44, Kurt Van Dijck wrote:
> > >On wo, 23 jan 2019 13:01:13 +0100, Oleksij Rempel wrote:
> > >>We only want to receive the package if src_name name packet's dest_name
> > >>is set and they are equal. If one name is missing we compare the
> > >>addresses instead (else branch).
> > >>
> > >>Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> > >>---
> > >> net/can/j1939/socket.c | 5 ++---
> > >> 1 file changed, 2 insertions(+), 3 deletions(-)
> > >>
> > >>diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
> > >>index 73ac92163373..9d3da573c31d 100644
> > >>--- a/net/can/j1939/socket.c
> > >>+++ b/net/can/j1939/socket.c
> > >>@@ -134,9 +134,8 @@ static void j1939_sk_recv_one(struct j1939_sock *jsk, struct sk_buff *oskb)
> > >> /* this socket does not take packets from this iface */
> > >> return;
> > >> if (!(jsk->state & J1939_SOCK_PROMISC)) {
> > >BTW, I assumed you had dropped promiscuous operation completely?
> >
> > why?
> because it made TP too complicated?
> The argument, if I remember well, is that spying on the bus can better
> be done using can-raw and a userspace j1939 stack?
> Anyway, that's not the point.
> >
> > >>- if (jsk->addr.src_name) {
> > >
> > >if src_name is set, then the user want to be addressed using its name.
> > >
> > >>- if (oskcb->addr.dst_name &&
> > >
> > >dst_name is set regardless of what the (receiving) user wants, it is set
> > >on available name.
> > >
> > >So this test actually is the good one, you don't want to fallback to
> > >addr comparison because you have not (yet) successfully claimed the
> > >addr, and hence, the recv part does not assign dst_name (yet).
> > >
> > >IMHO, you're breaking reception path during the address claiming
> > >process.
> > >Can you explain what problem you tried to solve here?
> >
> >
> > bind() with NAME no SA is set,
> > run AC and get SA 0x80
> >
> > get packet to 0x80, AC cache will interpolate NAME from address.
> >
> > so, packet will be passed.
> >
> > get packet to 0x81, no name was found in AC cache
> >
> > packet will be passed -- this looks wrong to me. Why should we pass packets
> > with different destination? Especially if socket was not configured to do
> > so.
>
> You're right!
> But this argument would justify to drop the test for dst_name
> completely.
> Because similarly, and empty dst_name does not justify to fallback to
> addr comparison either.
hm.. good points. I'll think about it.
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
