Hi Jason, Thanks for replying. Yeah, we can start looking into combining NetClient and CanBusClient in near future.
Meanwhile sending a V2 today with incorporating your suggestion.. Regards, Vikram > -----Original Message----- > From: Jason Wang <[email protected]> > Sent: Tuesday, February 25, 2020 7:25 PM > To: Vikram Garhwal <[email protected]>; [email protected] > Cc: Francisco Eduardo Iglesias <[email protected]> > Subject: Re: [PATCH 1/1] hw/net/can: Introduce Xlnx ZynqMP CAN > controller for QEMU > > > On 2020/2/25 下午2:22, Vikram Garhwal wrote: > > Hi Jason, > > Apologies for the delayed response. I tried plugging NetClientState in the > CAN which is required if we use qemu_send_packet but this will change the > underlying architecture of can-core, can-socketcan a lot. This means > changes the way CAN bus is created/works and socket CAN works. CAN > Socket(CAN Raw socket) is much different from Ethernet so plugging/using > NetClient state is not working here. > > > I get you. > > > > > > I apologize for still being a little confused about the filters but when > looking into the code, I can only find them being used with ethernet frames. > Since no other can controller uses NetClientState it makes me wonder if this > model perhaps was thought of being an ethernet NIC? > > > Nope NetclientState is not necessarily a NIC, it can be a peer of the NIC (e.g > network backend like tap, hubport etc). > > > > Or has the code in net/can/ which I referenced been obsoleted? > > > No :) > > > > > > Sharing this link for SocketCAN(in case you want to have a look): > https://www.kernel.org/doc/Documentation/networking/can.txt. Section 4 > talks on how CAN Socket is intended to work. Equivalent file is located as > net/can-socketcan.c. > > > Thanks for the pointer. > > I agree that there's no need to change that part. But we may consider to > unify the CanBusClientState and NetClientState in the future. > > > > > > Regards, > > Vikram > > > >> -----Original Message----- > >> From: Jason Wang <[email protected]> > >> Sent: Monday, February 10, 2020 7:09 PM > >> To: Vikram Garhwal <[email protected]>; [email protected] > >> Subject: Re: [PATCH 1/1] hw/net/can: Introduce Xlnx ZynqMP CAN > controller > >> for QEMU > >> > >> > >> On 2020/2/11 上午5:45, Vikram Garhwal wrote: > >>>>> + } > >>>>> + } else { > >>>>> + /* Normal mode Tx. */ > >>>>> + generate_frame(&frame, data); > >>>>> + > >>>>> + can_bus_client_send(&s->bus_client, &frame, 1); > >>>> I had a quick glance at can_bus_client_send(): > >>>> > >>>> It did: > >>>> > >>>> QTAILQ_FOREACH(peer, &bus->clients, next) { > >>>> if (peer->info->can_receive(peer)) { > >>>> if (peer == client) { > >>>> /* No loopback support for now */ > >>>> continue; > >>>> } > >>>> if (peer->info->receive(peer, frames, frames_cnt) > 0) { > >>>> ret = 1; > >>>> } > >>>> } > >>>> } > >>>> > >>>> which looks not correct. We need to use qemu_send_packet() instead > of > >>>> calling peer->info->receive() directly which bypasses filters > completely. > >>> [Vikram Garhwal] Can you please elaborate it bit more on why do we > need > >> to filter outgoing message? So, I can either add a filter before sending > the > >> packets. I am unable to understand the use case for it. For any message > which > >> is incoming, we are filtering it for sure before storing in > >> update_rx_fifo(). > >> > >> > >> I might be not clear, I meant the netfilters supported by qemu which > allows > >> you to attach a filter to a specific NetClientState, see > >> qemu_send_packet_async_with_flags. It doesn't mean the filter > implemented > >> in your own NIC model. > >> > >> Thanks > >> > >> > >>> Also, I can see existing CAN models like CAN sja1000 and CAN Kavser > are > >> using it same can_bus_client_send() function. However, this doesn't > mean > >> that it is the correct way to send & receive packets.
