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.
Kurt
