On Mon Jul 27 2020, Petr Machata wrote: > So this looks good, and works, but I'm wondering about one thing.
Thanks for testing. > > Your code (and evidently most drivers as well) use a different check > than mlxsw, namely skb->len + ETH_HLEN < X. When I print_hex_dump() > skb_mac_header(skb), skb->len in mlxsw with some test packet, I get e.g. > this: > > 00000000259a4db7: 01 00 5e 00 01 81 00 02 c9 a4 e4 e1 08 00 45 00 > ..^...........E. > 000000005f29f0eb: 00 48 0d c9 40 00 01 11 c8 59 c0 00 02 01 e0 00 > .H..@....Y...... > 00000000f3663e9e: 01 81 01 3f 01 3f 00 34 9f d3 00 02 00 2c 00 00 > ...?.?.4.....,.. > ^sp^^ ^dp^^ ^len^ ^cks^ ^len^ > 00000000b3914606: 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 > ................ > 000000002e7828ea: c9 ff fe a4 e4 e1 00 01 09 fa 00 00 00 00 00 00 > ................ > 000000000b98156e: 00 00 00 00 00 00 ...... > > Both UDP and PTP length fields indicate that the payload ends exactly at > the end of the dump. So apparently skb->len contains all the payload > bytes, including the Ethernet header. > > Is that the case for other drivers as well? Maybe mlxsw is just missing > some SKB magic in the driver. So I run some tests (on other hardware/drivers) and it seems like that the skb->len usually doesn't include the ETH_HLEN. Therefore, it is added to the check. Looking at the driver code: |static void mlxsw_sp_rx_sample_listener(struct sk_buff *skb, u8 local_port, | void *trap_ctx) |{ | [...] | /* The sample handler expects skb->data to point to the start of the | * Ethernet header. | */ | skb_push(skb, ETH_HLEN); | mlxsw_sp_sample_receive(mlxsw_sp, skb, local_port); |} Maybe that's the issue here? I was also wondering about something else in that driver driver: The parsing code allows for ptp v1, but the message type was always fetched from offset 0 in the header. Is that indented? Thanks, Kurt
signature.asc
Description: PGP signature