Re: [PATCH] kselftest: alsa: fixed a print formatting warning
On Sun, 17 Dec 2023 09:00:19 +0100, Ghanshyam Agrawal wrote: > > A statement used %d print formatter where %s should have > been used. The same has been fixed in this commit. > > Signed-off-by: Ghanshyam Agrawal Thanks, applied now. Takashi
Re: [PATCH v8 23/24] ima: Make it independent from 'integrity' LSM
On Thu, 2023-12-14 at 18:08 +0100, Roberto Sassu wrote:
> From: Roberto Sassu
>
> Make the 'ima' LSM independent from the 'integrity' LSM by introducing IMA
> own integrity metadata (ima_iint_cache structure, with IMA-specific fields
> from the integrity_iint_cache structure), and by managing it directly from
> the 'ima' LSM.
>
> Move the remaining IMA-specific flags to security/integrity/ima/ima.h,
> since they are now unnecessary in the common integrity layer.
>
> Replace integrity_iint_cache with ima_iint_cache in various places
> of the IMA code.
>
> Then, reserve space in the security blob for the entire ima_iint_cache
> structure, so that it is available for all inodes having the security blob
> allocated (those for which security_inode_alloc() was called). Adjust the
> IMA code accordingly, call ima_iint_inode() to retrieve the ima_iint_cache
> structure. Keep the non-NULL checks since there can be inodes without
> security blob.
>
> Don't include the inode pointer as field in the ima_iint_cache structure,
> since the association with the inode is clear. Since the inode field is
> missing in ima_iint_cache, pass the extra inode parameter to
> ima_get_verity_digest().
>
> Finally, register ima_inode_alloc_security/ima_inode_free_security() to
> initialize/deinitialize the new ima_iint_cache structure (before this task
> was done by iint_init_always() and iint_free()). Also, duplicate
> iint_lockdep_annotate() for the ima_iint_cache structure, and name it
> ima_iint_lockdep_annotate().
>
> Signed-off-by: Roberto Sassu
> ---
> security/integrity/ima/ima.h | 109 ++
> security/integrity/ima/ima_api.c | 15 ++--
> security/integrity/ima/ima_appraise.c | 25 +++---
> security/integrity/ima/ima_init.c | 2 +-
> security/integrity/ima/ima_main.c | 96 ++-
> security/integrity/ima/ima_policy.c | 2 +-
> security/integrity/integrity.h| 53 -
> 7 files changed, 194 insertions(+), 108 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index a27fc10f84f7..d1c339a340f8 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -60,7 +60,7 @@ extern const char boot_aggregate_name[];
>
> /* IMA event related data */
> struct ima_event_data {
> - struct integrity_iint_cache *iint;
> + struct ima_iint_cache *iint;
> struct file *file;
> const unsigned char *filename;
> struct evm_ima_xattr_data *xattr_value;
> @@ -119,6 +119,86 @@ struct ima_kexec_hdr {
> u64 count;
> };
>
> +/* IMA iint action cache flags */
> +#define IMA_MEASURE 0x0001
> +#define IMA_MEASURED 0x0002
> +#define IMA_APPRAISE 0x0004
> +#define IMA_APPRAISED0x0008
> +/*#define IMA_COLLECT0x0010 do not use this flag */
> +#define IMA_COLLECTED0x0020
> +#define IMA_AUDIT0x0040
> +#define IMA_AUDITED 0x0080
> +#define IMA_HASH 0x0100
> +#define IMA_HASHED 0x0200
> +
> +/* IMA iint policy rule cache flags */
> +#define IMA_NONACTION_FLAGS 0xff00
> +#define IMA_DIGSIG_REQUIRED 0x0100
> +#define IMA_PERMIT_DIRECTIO 0x0200
> +#define IMA_NEW_FILE 0x0400
> +#define IMA_FAIL_UNVERIFIABLE_SIGS 0x1000
> +#define IMA_MODSIG_ALLOWED 0x2000
> +#define IMA_CHECK_BLACKLIST 0x4000
> +#define IMA_VERITY_REQUIRED 0x8000
> +
> +#define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
> + IMA_HASH | IMA_APPRAISE_SUBMASK)
> +#define IMA_DONE_MASK(IMA_MEASURED | IMA_APPRAISED |
> IMA_AUDITED | \
> + IMA_HASHED | IMA_COLLECTED | \
> + IMA_APPRAISED_SUBMASK)
> +
> +/* IMA iint subaction appraise cache flags */
> +#define IMA_FILE_APPRAISE0x1000
> +#define IMA_FILE_APPRAISED 0x2000
> +#define IMA_MMAP_APPRAISE0x4000
> +#define IMA_MMAP_APPRAISED 0x8000
> +#define IMA_BPRM_APPRAISE0x0001
> +#define IMA_BPRM_APPRAISED 0x0002
> +#define IMA_READ_APPRAISE0x0004
> +#define IMA_READ_APPRAISED 0x0008
> +#define IMA_CREDS_APPRAISE 0x0010
> +#define IMA_CREDS_APPRAISED 0x0020
> +#define IMA_APPRAISE_SUBMASK (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
> + IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
> + IMA_CREDS_APPRAISE)
> +#define IMA_APPRAISED_SUBMASK(IMA_FILE_APPRAISED |
> IMA_MMAP_APPRAISED | \
> + IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
> + IMA_CREDS_APPRAISED)
> +
> +/* IMA iint cache atomic_flags */
> +#define IMA_CHANGE_XATTR 0
> +#define IMA_UPDATE_XATTR 1
> +#define IMA_CHANGE_ATTR 2
> +#define IMA_DIGSIG 3
> +#define IMA_MUST
Re: [PATCH 02/12] selftests/net: Verify that TCP-AO complies with ignoring ICMPs
On Fri, Dec 15, 2023 at 02:36:16AM +, Dmitry Safonov wrote: > Hand-crafted ICMP packets are sent to the server, the server checks for > hard/soft errors and fails if any. > > Expected output for ipv4 version: > > # ./icmps-discard_ipv4 > > 1..3 > > # 3164[lib/setup.c:166] rand seed 1642623745 > > TAP version 13 > > # 3164[lib/proc.c:207]Snmp6 Ip6InReceives: 0 => 1 > > # 3164[lib/proc.c:207]Snmp6 Ip6InNoRoutes: 0 => 1 > > # 3164[lib/proc.c:207]Snmp6 Ip6InOctets: 0 => 76 > > # 3164[lib/proc.c:207]Snmp6Ip6InNoECTPkts: 0 => 1 > > # 3164[lib/proc.c:207] TcpInSegs: 2 => 203 > > # 3164[lib/proc.c:207] Tcp OutSegs: 1 => 202 > > # 3164[lib/proc.c:207] IcmpMsg InType3: 0 => 543 > > # 3164[lib/proc.c:207] IcmpInMsgs: 0 => 543 > > # 3164[lib/proc.c:207] IcmpInDestUnreachs: 0 => 543 > > # 3164[lib/proc.c:207] IpInReceives: 2 => 746 > > # 3164[lib/proc.c:207] IpInDelivers: 2 => 746 > > # 3164[lib/proc.c:207] Ip OutRequests: 1 => 202 > > # 3164[lib/proc.c:207]IpExt InOctets: 132 => 61684 > > # 3164[lib/proc.c:207]IpExt OutOctets: 68 => 31324 > > # 3164[lib/proc.c:207]IpExt InNoECTPkts: 2 => 744 > > # 3164[lib/proc.c:207] TcpExt TCPPureAcks: 1 => 2 > > # 3164[lib/proc.c:207] TcpExt TCPOrigDataSent: 0 => 200 > > # 3164[lib/proc.c:207] TcpExt TCPDelivered: 0 => 199 > > # 3164[lib/proc.c:207] TcpExt TCPAOGood: 2 => 203 > > # 3164[lib/proc.c:207] TcpExt TCPAODroppedIcmps: 0 => 541 > > ok 1 InDestUnreachs delivered 543 > > ok 2 Server survived 2 bytes of traffic > > ok 3 ICMPs ignored 541 > > # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Expected output for ipv6 version: > > # ./icmps-discard_ipv6 > > 1..3 > > # 3186[lib/setup.c:166] rand seed 1642623803 > > TAP version 13 > > # 3186[lib/proc.c:207]Snmp6 Ip6InReceives: 4 => 568 > > # 3186[lib/proc.c:207]Snmp6 Ip6InDelivers: 3 => 564 > > # 3186[lib/proc.c:207]Snmp6Ip6OutRequests: 2 => 204 > > # 3186[lib/proc.c:207]Snmp6Ip6InMcastPkts: 1 => 4 > > # 3186[lib/proc.c:207]Snmp6 Ip6OutMcastPkts: 0 => 1 > > # 3186[lib/proc.c:207]Snmp6 Ip6InOctets: 320 => 70420 > > # 3186[lib/proc.c:207]Snmp6 Ip6OutOctets: 160 => 35512 > > # 3186[lib/proc.c:207]Snmp6 Ip6InMcastOctets: 72 => 336 > > # 3186[lib/proc.c:207]Snmp6 Ip6OutMcastOctets: 0 => 76 > > # 3186[lib/proc.c:207]Snmp6Ip6InNoECTPkts: 4 => 568 > > # 3186[lib/proc.c:207]Snmp6 Icmp6InMsgs: 1 => 361 > > # 3186[lib/proc.c:207]Snmp6 Icmp6OutMsgs: 1 => 2 > > # 3186[lib/proc.c:207]Snmp6 Icmp6InDestUnreachs: 0 => 360 > > # 3186[lib/proc.c:207]Snmp6 Icmp6OutMLDv2Reports: 0 => 1 > > # 3186[lib/proc.c:207]Snmp6 Icmp6InType1: 0 => 360 > > # 3186[lib/proc.c:207]Snmp6 Icmp6OutType143: 0 => 1 > > # 3186[lib/proc.c:207] TcpInSegs: 2 => 203 > > # 3186[lib/proc.c:207] Tcp OutSegs: 1 => 202 > > # 3186[lib/proc.c:207] TcpExt TCPPureAcks: 1 => 2 > > # 3186[lib/proc.c:207] TcpExt TCPOrigDataSent: 0 => 200 > > # 3186[lib/proc.c:207] TcpExt TCPDelivered: 0 => 199 > > # 3186[lib/proc.c:207] TcpExt TCPAOGood: 2 => 203 > > # 3186[lib/proc.c:207] TcpExt TCPAODroppedIcmps: 0 => 360 > > ok 1 Icmp6InDestUnreachs delivered 360 > > ok 2 Server survived 2 bytes of traffic > > ok 3 ICMPs ignored 360 > > # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Signed-off-by: Dmitry Safonov Tested-by: Hangbin Liu
Re: [PATCH 03/12] selftests/net: Add TCP-AO ICMPs accept test
On Fri, Dec 15, 2023 at 02:36:17AM +, Dmitry Safonov wrote: > Reverse to icmps-discard test: the server accepts ICMPs, using > TCP_AO_CMDF_ACCEPT_ICMP and it is expected to fail under ICMP > flood from client. Test that the default pre-TCP-AO behaviour functions > when TCP_AO_CMDF_ACCEPT_ICMP is set. > > Expected output for ipv4 version (in case it receives ICMP_PROT_UNREACH): > > # ./icmps-accept_ipv4 > > 1..3 > > # 3209[lib/setup.c:166] rand seed 1642623870 > > TAP version 13 > > # 3209[lib/proc.c:207]Snmp6 Ip6InReceives: 0 => 1 > > # 3209[lib/proc.c:207]Snmp6 Ip6InNoRoutes: 0 => 1 > > # 3209[lib/proc.c:207]Snmp6 Ip6InOctets: 0 => 76 > > # 3209[lib/proc.c:207]Snmp6Ip6InNoECTPkts: 0 => 1 > > # 3209[lib/proc.c:207] TcpInSegs: 3 => 23 > > # 3209[lib/proc.c:207] Tcp OutSegs: 2 => 22 > > # 3209[lib/proc.c:207] IcmpMsg InType3: 0 => 4 > > # 3209[lib/proc.c:207] IcmpInMsgs: 0 => 4 > > # 3209[lib/proc.c:207] IcmpInDestUnreachs: 0 => 4 > > # 3209[lib/proc.c:207] IpInReceives: 3 => 27 > > # 3209[lib/proc.c:207] IpInDelivers: 3 => 27 > > # 3209[lib/proc.c:207] Ip OutRequests: 2 => 22 > > # 3209[lib/proc.c:207]IpExt InOctets: 288 => 3420 > > # 3209[lib/proc.c:207]IpExt OutOctets: 124 => 3244 > > # 3209[lib/proc.c:207]IpExt InNoECTPkts: 3 => 25 > > # 3209[lib/proc.c:207] TcpExt TCPPureAcks: 1 => 2 > > # 3209[lib/proc.c:207] TcpExt TCPOrigDataSent: 0 => 20 > > # 3209[lib/proc.c:207] TcpExt TCPDelivered: 0 => 19 > > # 3209[lib/proc.c:207] TcpExt TCPAOGood: 3 => 23 > > ok 1 InDestUnreachs delivered 4 > > ok 2 server failed with -92: Protocol not available > > ok 3 TCPAODroppedIcmps counter didn't change: 0 >= 0 > > # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Expected output for ipv6 version (in case it receives ADM_PROHIBITED): > > # ./icmps-accept_ipv6 > > 1..3 > > # 3277[lib/setup.c:166] rand seed 1642624035 > > TAP version 13 > > # 3277[lib/proc.c:207]Snmp6 Ip6InReceives: 6 => 31 > > # 3277[lib/proc.c:207]Snmp6 Ip6InDelivers: 4 => 29 > > # 3277[lib/proc.c:207]Snmp6Ip6OutRequests: 4 => 24 > > # 3277[lib/proc.c:207]Snmp6 Ip6InOctets: 592 => 4492 > > # 3277[lib/proc.c:207]Snmp6 Ip6OutOctets: 332 => 3852 > > # 3277[lib/proc.c:207]Snmp6Ip6InNoECTPkts: 6 => 31 > > # 3277[lib/proc.c:207]Snmp6 Icmp6InMsgs: 1 => 6 > > # 3277[lib/proc.c:207]Snmp6 Icmp6InDestUnreachs: 0 => 5 > > # 3277[lib/proc.c:207]Snmp6 Icmp6InType1: 0 => 5 > > # 3277[lib/proc.c:207] TcpInSegs: 3 => 23 > > # 3277[lib/proc.c:207] Tcp OutSegs: 2 => 22 > > # 3277[lib/proc.c:207] TcpExt TCPPureAcks: 1 => 2 > > # 3277[lib/proc.c:207] TcpExt TCPOrigDataSent: 0 => 20 > > # 3277[lib/proc.c:207] TcpExt TCPDelivered: 0 => 19 > > # 3277[lib/proc.c:207] TcpExt TCPAOGood: 3 => 23 > > ok 1 Icmp6InDestUnreachs delivered 5 > > ok 2 server failed with -13: Permission denied > > ok 3 TCPAODroppedIcmps counter didn't change: 0 >= 0 > > # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0 > > With some luck the server may fail with ECONNREFUSED (depending on what > icmp packet was delivered firstly). > For the kernel error handlers see: tab_unreach[] and icmp_err_convert[]. > > Signed-off-by: Dmitry Safonov Tested-by: Hangbin Liu
Re: [PATCH 04/12] selftests/net: Add a test for TCP-AO keys matching
On Fri, Dec 15, 2023 at 02:36:18AM +, Dmitry Safonov wrote: > Add TCP-AO tests on connect()/accept() pair. > SNMP counters exposed by kernel are very useful here to verify the > expected behavior of TCP-AO. > > Expected output for ipv4 version: > > # ./connect-deny_ipv4 > > 1..19 > > # 1702[lib/setup.c:254] rand seed 1680553689 > > TAP version 13 > > ok 1 Non-AO server + AO client > > ok 2 Non-AO server + AO client: counter TCPAOKeyNotFound increased 0 => 1 > > ok 3 AO server + Non-AO client > > ok 4 AO server + Non-AO client: counter TCPAORequired increased 0 => 1 > > ok 5 Wrong password > > ok 6 Wrong password: counter TCPAOBad increased 0 => 1 > > ok 7 Wrong rcv id > > ok 8 Wrong rcv id: counter TCPAOKeyNotFound increased 1 => 2 > > ok 9 Wrong snd id > > ok 10 Wrong snd id: counter TCPAOGood increased 0 => 1 > > ok 11 Server: Wrong addr: counter TCPAOKeyNotFound increased 2 => 3 > > ok 12 Server: Wrong addr > > ok 13 Client: Wrong addr: connect() was prevented > > ok 14 rcv id != snd id: connected > > ok 15 rcv id != snd id: counter TCPAOGood increased 1 => 3 > > ok 16 Server: prefix match: connected > > ok 17 Server: prefix match: counter TCPAOGood increased 4 => 6 > > ok 18 Client: prefix match: connected > > ok 19 Client: prefix match: counter TCPAOGood increased 7 => 9 > > # Totals: pass:19 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Expected output for ipv6 version: > > # ./connect-deny_ipv6 > > 1..19 > > # 1725[lib/setup.c:254] rand seed 1680553711 > > TAP version 13 > > ok 1 Non-AO server + AO client > > ok 2 Non-AO server + AO client: counter TCPAOKeyNotFound increased 0 => 1 > > ok 3 AO server + Non-AO client: counter TCPAORequired increased 0 => 1 > > ok 4 AO server + Non-AO client > > ok 5 Wrong password: counter TCPAOBad increased 0 => 1 > > ok 6 Wrong password > > ok 7 Wrong rcv id: counter TCPAOKeyNotFound increased 1 => 2 > > ok 8 Wrong rcv id > > ok 9 Wrong snd id: counter TCPAOGood increased 0 => 1 > > ok 10 Wrong snd id > > ok 11 Server: Wrong addr > > ok 12 Server: Wrong addr: counter TCPAOKeyNotFound increased 2 => 3 > > ok 13 Client: Wrong addr: connect() was prevented > > ok 14 rcv id != snd id: connected > > ok 15 rcv id != snd id: counter TCPAOGood increased 1 => 3 > > ok 16 Server: prefix match: connected > > ok 17 Server: prefix match: counter TCPAOGood increased 5 => 7 > > ok 18 Client: prefix match: connected > > ok 19 Client: prefix match: counter TCPAOGood increased 8 => 10 > > # Totals: pass:19 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Signed-off-by: Dmitry Safonov Tested-by: Hangbin Liu
Re: [PATCH 05/12] selftests/net: Add test for TCP-AO add setsockopt() command
On Fri, Dec 15, 2023 at 02:36:19AM +, Dmitry Safonov wrote: > Verify corner-cases for UAPI. > Sample output: > > # ./setsockopt-closed_ipv4 > > 1..120 > > # 1657[lib/setup.c:254] rand seed 1681938184 > > TAP version 13 > > ok 1 AO add: minimum size > > ok 2 AO add: extended size > > ok 3 AO add: null optval > > ok 4 AO del: minimum size > > ok 5 AO del: extended size > > ok 6 AO del: null optval > > ok 7 AO set info: minimum size > > ok 8 AO set info: extended size > > ok 9 AO info get: : extended size > > ok 10 AO set info: null optval > > ok 11 AO get info: minimum size > > ok 12 AO get info: extended size > > ok 13 AO get info: null optval > > ok 14 AO get info: null optlen > > ok 15 AO get keys: minimum size > > ok 16 AO get keys: extended size > > ok 17 AO get keys: null optval > > ok 18 AO get keys: null optlen > > ok 19 key add: too big keylen > > ok 20 key add: using reserved padding > > ok 21 key add: using reserved2 padding > > ok 22 key add: wrong address family > > ok 23 key add: port (unsupported) > > ok 24 key add: no prefix, addr > > ok 25 key add: no prefix, any addr > > ok 26 key add: prefix, any addr > > ok 27 key add: too big prefix > > ok 28 key add: too short prefix > > ok 29 key add: bad key flags > > ok 30 key add: add current key on a listen socket > > ok 31 key add: add rnext key on a listen socket > > ok 32 key add: add current+rnext key on a listen socket > > ok 33 key add: add key and set as current > > ok 34 key add: add key and set as rnext > > ok 35 key add: add key and set as current+rnext > > ok 36 key add: ifindex without TCP_AO_KEYF_IFNINDEX > > ok 37 key add: non-existent VRF > > ok 38 optmem limit was hit on adding 69 key > > ok 39 key add: maclen bigger than TCP hdr > > ok 40 key add: bad algo > > ok 41 key del: using reserved padding > > ok 42 key del: using reserved2 padding > > ok 43 key del: del and set current key on a listen socket > > ok 44 key del: del and set rnext key on a listen socket > > ok 45 key del: del and set current+rnext key on a listen socket > > ok 46 key del: bad key flags > > ok 47 key del: ifindex without TCP_AO_KEYF_IFNINDEX > > ok 48 key del: non-existent VRF > > ok 49 key del: set non-exising current key > > ok 50 key del: set non-existing rnext key > > ok 51 key del: set non-existing current+rnext key > > ok 52 key del: set current key > > ok 53 key del: set rnext key > > ok 54 key del: set current+rnext key > > ok 55 key del: set as current key to be removed > > ok 56 key del: set as rnext key to be removed > > ok 57 key del: set as current+rnext key to be removed > > ok 58 key del: async on non-listen > > ok 59 key del: non-existing sndid > > ok 60 key del: non-existing rcvid > > ok 61 key del: incorrect addr > > ok 62 key del: correct key delete > > ok 63 AO info set: set current key on a listen socket > > ok 64 AO info set: set rnext key on a listen socket > > ok 65 AO info set: set current+rnext key on a listen socket > > ok 66 AO info set: using reserved padding > > ok 67 AO info set: using reserved2 padding > > ok 68 AO info set: accept_icmps > > ok 69 AO info get: accept_icmps > > ok 70 AO info set: ao required > > ok 71 AO info get: ao required > > ok 72 AO info set: ao required with MD5 key > > ok 73 AO info set: set non-existing current key > > ok 74 AO info set: set non-existing rnext key > > ok 75 AO info set: set non-existing current+rnext key > > ok 76 AO info set: set current key > > ok 77 AO info get: set current key > > ok 78 AO info set: set rnext key > > ok 79 AO info get: set rnext key > > ok 80 AO info set: set current+rnext key > > ok 81 AO info get: set current+rnext key > > ok 82 AO info set: set counters > > ok 83 AO info get: set counters > > ok 84 AO info set: no-op > > ok 85 AO info get: no-op > > ok 86 get keys: no ao_info > > ok 87 get keys: proper tcp_ao_get_mkts() > > ok 88 get keys: set out-only pkt_good counter > > ok 89 get keys: set out-only pkt_bad counter > > ok 90 get keys: bad keyflags > > ok 91 get keys: ifindex without TCP_AO_KEYF_IFNINDEX > > ok 92 get keys: using reserved field > > ok 93 get keys: no prefix, addr > > ok 94 get keys: no prefix, any addr > > ok 95 get keys: prefix, any addr > > ok 96 get keys: too big prefix > > ok 97 get keys: too short prefix > > ok 98 get keys: prefix + addr > > ok 99 get keys: get_all + prefix > > ok 100 get keys: get_all + addr > > ok 101 get keys: get_all + sndid > > ok 102 get keys: get_all + rcvid > > ok 103 get keys: current + prefix > > ok 104 get keys: current + addr > > ok 105 get keys: current + sndid > > ok 106 get keys: current + rcvid > > ok 107 get keys: rnext + prefix > > ok 108 get keys: rnext + addr > > ok 109 get keys: rnext + sndid > > ok 110 get keys: rnext + rcvid > > ok 111 get keys: get_all + current > > ok 112 get keys: get_all + rnext > > ok 113 get keys: current + rnext > > ok 114 key add: duplicate: full copy > > ok 115 key add: duplicate: any addr key on the socket > > ok 116 key add: duplicate: add any addr key > > ok 117 key add
Re: [PATCH 06/12] selftests/net: Add TCP-AO + TCP-MD5 + no sign listen socket tests
Hi Dmitry, With 6.7.0-rc3 I always got error # selftests: net/tcp_ao: unsigned-md5_ipv6 # 1..72 # # 1577[lib/setup.c:243] rand seed 1702890409 # TAP version 13 # ok 1 AO server (INADDR_ANY): AO client: counter TCPAOGood increased 0 => 2 # ok 2 AO server (INADDR_ANY): AO client: connected # ok 3 AO server (INADDR_ANY): MD5 client ... # ok 52 TCP-MD5 key intersects with existing TCP-AO key: prefailed as expected: Key was rejected by service # ok 53 TCP-MD5 key + TCP-AO required: prefailed as expected: Key was rejected by service # ok 54 TCP-AO required on socket + TCP-MD5 key: prefailed as expected: Key was rejected by service # not ok 55 # error 1578[unsigned-md5.c:33] Failed to add a route to VRF: Key was rejected by service # # Planned tests != run tests (72 != 55) # # Totals: pass:54 fail:0 xfail:0 xpass:0 skip:0 error:1 ok 1 selftests: net/tcp_ao: unsigned-md5_ipv6 Do you have any idea? Thanks Hangbin On Fri, Dec 15, 2023 at 02:36:20AM +, Dmitry Safonov wrote: > The test plan was (most of tests have all 3 client types): > 1. TCP-AO listen (INADDR_ANY) > 2. TCP-MD5 listen (INADDR_ANY) > 3. non-signed listen (INADDR_ANY) > 4. TCP-AO + TCP-MD5 listen (prefix) > 5. TCP-AO subprefix add failure [checked in setsockopt-closed.c] > 6. TCP-AO out of prefix connect [checked in connect-deny.c] > 7. TCP-AO + TCP-MD5 on connect() > 8. TCP-AO intersect with TCP-MD5 failure > 9. Established TCP-AO: add TCP-MD5 key > 10. Established TCP-MD5: add TCP-AO key > 11. Established non-signed: add TCP-AO key > > Output produced: > > # ./unsigned-md5_ipv6 > > 1..72 > > # 1592[lib/setup.c:239] rand seed 1697567046 > > TAP version 13 > > ok 1 AO server (INADDR_ANY): AO client: counter TCPAOGood increased 0 => 2 > > ok 2 AO server (INADDR_ANY): AO client: connected > > ok 3 AO server (INADDR_ANY): MD5 client > > ok 4 AO server (INADDR_ANY): MD5 client: counter TCPMD5Unexpected increased > > 0 => 1 > > ok 5 AO server (INADDR_ANY): no sign client: counter TCPAORequired > > increased 0 => 1 > > ok 6 AO server (INADDR_ANY): unsigned client > > ok 7 AO server (AO_REQUIRED): AO client: connected > > ok 8 AO server (AO_REQUIRED): AO client: counter TCPAOGood increased 4 => 6 > > ok 9 AO server (AO_REQUIRED): unsigned client > > ok 10 AO server (AO_REQUIRED): unsigned client: counter TCPAORequired > > increased 1 => 2 > > ok 11 MD5 server (INADDR_ANY): AO client: counter TCPAOKeyNotFound > > increased 0 => 1 > > ok 12 MD5 server (INADDR_ANY): AO client > > ok 13 MD5 server (INADDR_ANY): MD5 client: connected > > ok 14 MD5 server (INADDR_ANY): MD5 client: no counter checks > > ok 15 MD5 server (INADDR_ANY): no sign client > > ok 16 MD5 server (INADDR_ANY): no sign client: counter TCPMD5NotFound > > increased 0 => 1 > > ok 17 no sign server: AO client > > ok 18 no sign server: AO client: counter TCPAOKeyNotFound increased 1 => 2 > > ok 19 no sign server: MD5 client > > ok 20 no sign server: MD5 client: counter TCPMD5Unexpected increased 1 => 2 > > ok 21 no sign server: no sign client: connected > > ok 22 no sign server: no sign client: counter CurrEstab increased 0 => 1 > > ok 23 AO+MD5 server: AO client (matching): connected > > ok 24 AO+MD5 server: AO client (matching): counter TCPAOGood increased 8 => > > 10 > > ok 25 AO+MD5 server: AO client (misconfig, matching MD5) > > ok 26 AO+MD5 server: AO client (misconfig, matching MD5): counter > > TCPAOKeyNotFound increased 2 => 3 > > ok 27 AO+MD5 server: AO client (misconfig, non-matching): counter > > TCPAOKeyNotFound increased 3 => 4 > > ok 28 AO+MD5 server: AO client (misconfig, non-matching) > > ok 29 AO+MD5 server: MD5 client (matching): connected > > ok 30 AO+MD5 server: MD5 client (matching): no counter checks > > ok 31 AO+MD5 server: MD5 client (misconfig, matching AO) > > ok 32 AO+MD5 server: MD5 client (misconfig, matching AO): counter > > TCPMD5Unexpected increased 2 => 3 > > ok 33 AO+MD5 server: MD5 client (misconfig, non-matching) > > ok 34 AO+MD5 server: MD5 client (misconfig, non-matching): counter > > TCPMD5Unexpected increased 3 => 4 > > ok 35 AO+MD5 server: no sign client (unmatched): connected > > ok 36 AO+MD5 server: no sign client (unmatched): counter CurrEstab > > increased 0 => 1 > > ok 37 AO+MD5 server: no sign client (misconfig, matching AO) > > ok 38 AO+MD5 server: no sign client (misconfig, matching AO): counter > > TCPAORequired increased 2 => 3 > > ok 39 AO+MD5 server: no sign client (misconfig, matching MD5) > > ok 40 AO+MD5 server: no sign client (misconfig, matching MD5): counter > > TCPMD5NotFound increased 1 => 2 > > ok 41 AO+MD5 server: client with both [TCP-MD5] and TCP-AO keys: connect() > > was prevented > > ok 42 AO+MD5 server: client with both [TCP-MD5] and TCP-AO keys: no counter > > checks > > ok 43 AO+MD5 server: client with both TCP-MD5 and [TCP-AO] keys: connect() > > was prevented > > ok 44 AO+MD5 server: client with both TCP-MD5 and [TCP-AO] keys: no counter > > checks > > ok 45 TCP-AO established: add TCP-MD5 key: po
Re: [PATCH 07/12] selftests/net: Add test/benchmark for removing MKTs
On Fri, Dec 15, 2023 at 02:36:21AM +, Dmitry Safonov wrote: > Sample output: > > 1..36 > > # 1106[lib/setup.c:207] rand seed 1660754406 > > TAP version 13 > > ok 1 Worst case connect 512 keys: min=0ms max=1ms mean=0.583329ms > > stddev=0.076376 > > ok 2 Connect random-search 512 keys: min=0ms max=1ms mean=0.53412ms > > stddev=0.0516779 > > ok 3Worst case delete 512 keys: min=2ms max=11ms mean=6.04139ms > > stddev=0.245792 > > ok 4Add a new key 512 keys: min=0ms max=13ms mean=0.673415ms > > stddev=0.0820618 > > ok 5 Remove random-search 512 keys: min=5ms max=9ms mean=6.65969ms > > stddev=0.258064 > > ok 6 Remove async 512 keys: min=0ms max=0ms mean=0.041825ms > > stddev=0.0204512 > > ok 7 Worst case connect 1024 keys: min=0ms max=2ms mean=0.520357ms > > stddev=0.0721358 > > ok 8 Connect random-search 1024 keys: min=0ms max=2ms mean=0.535312ms > > stddev=0.0517355 > > ok 9Worst case delete 1024 keys: min=5ms max=9ms mean=8.27219ms > > stddev=0.287614 > > ok 10Add a new key 1024 keys: min=0ms max=1ms mean=0.688121ms > > stddev=0.0829531 > > ok 11 Remove random-search 1024 keys: min=5ms max=9ms mean=8.37649ms > > stddev=0.289422 > > ok 12 Remove async 1024 keys: min=0ms max=0ms mean=0.0457096ms > > stddev=0.0213798 > > ok 13 Worst case connect 2048 keys: min=0ms max=2ms mean=0.748804ms > > stddev=0.0865335 > > ok 14 Connect random-search 2048 keys: min=0ms max=2ms mean=0.782993ms > > stddev=0.0625697 > > ok 15Worst case delete 2048 keys: min=5ms max=10ms mean=8.23106ms > > stddev=0.286898 > > ok 16Add a new key 2048 keys: min=0ms max=1ms mean=0.812988ms > > stddev=0.0901658 > > ok 17 Remove random-search 2048 keys: min=8ms max=9ms mean=8.84949ms > > stddev=0.297481 > > ok 18 Remove async 2048 keys: min=0ms max=0ms mean=0.0297223ms > > stddev=0.0172402 > > ok 19 Worst case connect 4096 keys: min=1ms max=5ms mean=1.53352ms > > stddev=0.123836 > > ok 20 Connect random-search 4096 keys: min=1ms max=5ms mean=1.52226ms > > stddev=0.0872429 > > ok 21Worst case delete 4096 keys: min=5ms max=9ms mean=8.25874ms > > stddev=0.28738 > > ok 22Add a new key 4096 keys: min=0ms max=3ms mean=1.67382ms > > stddev=0.129376 > > ok 23 Remove random-search 4096 keys: min=5ms max=10ms mean=8.26178ms > > stddev=0.287433 > > ok 24 Remove async 4096 keys: min=0ms max=0ms mean=0.0340009ms > > stddev=0.0184393 > > ok 25 Worst case connect 8192 keys: min=2ms max=4ms mean=2.86208ms > > stddev=0.169177 > > ok 26 Connect random-search 8192 keys: min=2ms max=4ms mean=2.87592ms > > stddev=0.119915 > > ok 27Worst case delete 8192 keys: min=6ms max=11ms mean=7.55291ms > > stddev=0.274826 > > ok 28Add a new key 8192 keys: min=1ms max=5ms mean=2.56797ms > > stddev=0.160249 > > ok 29 Remove random-search 8192 keys: min=5ms max=10ms mean=7.14002ms > > stddev=0.267208 > > ok 30 Remove async 8192 keys: min=0ms max=0ms mean=0.0320066ms > > stddev=0.0178904 > > ok 31 Worst case connect 16384 keys: min=5ms max=6ms mean=5.55334ms > > stddev=0.235655 > > ok 32 Connect random-search 16384 keys: min=5ms max=6ms mean=5.52614ms > > stddev=0.166225 > > ok 33Worst case delete 16384 keys: min=5ms max=11ms mean=7.39109ms > > stddev=0.271866 > > ok 34Add a new key 16384 keys: min=2ms max=4ms mean=3.35799ms > > stddev=0.183248 > > ok 35 Remove random-search 16384 keys: min=5ms max=8ms mean=6.86078ms > > stddev=0.261931 > > ok 36 Remove async 16384 keys: min=0ms max=0ms > > mean=0.0302384ms stddev=0.0173892 > > # Totals: pass:36 fail:0 xfail:0 xpass:0 skip:0 error:0 > > From the output it's visible that the current simplified approach with > linked-list of MKTs scales quite fine even for thousands of keys. > And that also means that the majority of the time for delete is eaten by > synchronize_rcu() [which I can confirm separately by tracing]. > > Signed-off-by: Dmitry Safonov Tested-by: Hangbin Liu
Re: [PATCH 08/12] selftests/net: Add TCP_REPAIR TCP-AO tests
On Fri, Dec 15, 2023 at 02:36:22AM +, Dmitry Safonov wrote: > The test plan is: > 1. check that TCP-AO connection may be restored on another socket > 2. check restore with wrong send/recv ISN (checking that they are >part of MAC generation) > 3. check restore with wrong SEQ number extension (checking that >high bytes of it taken into MAC generation) > > Sample output expected: > > # ./restore_ipv4 > > 1..20 > > # 1412[lib/setup.c:254] rand seed 1686610825 > > TAP version 13 > > ok 1 TCP-AO migrate to another socket: server alive > > ok 2 TCP-AO migrate to another socket: post-migrate connection is alive > > ok 3 TCP-AO migrate to another socket: counter TCPAOGood increased 23 => 44 > > ok 4 TCP-AO migrate to another socket: counter TCPAOGood increased 22 => 42 > > ok 5 TCP-AO with wrong send ISN: server couldn't serve > > ok 6 TCP-AO with wrong send ISN: post-migrate connection is broken > > ok 7 TCP-AO with wrong send ISN: counter TCPAOBad increased 0 => 4 > > ok 8 TCP-AO with wrong send ISN: counter TCPAOBad increased 0 => 3 > > ok 9 TCP-AO with wrong receive ISN: server couldn't serve > > ok 10 TCP-AO with wrong receive ISN: post-migrate connection is broken > > ok 11 TCP-AO with wrong receive ISN: counter TCPAOBad increased 4 => 8 > > ok 12 TCP-AO with wrong receive ISN: counter TCPAOBad increased 5 => 10 > > ok 13 TCP-AO with wrong send SEQ ext number: server couldn't serve > > ok 14 TCP-AO with wrong send SEQ ext number: post-migrate connection is > > broken > > ok 15 TCP-AO with wrong send SEQ ext number: counter TCPAOBad increased 9 > > => 10 > > ok 16 TCP-AO with wrong send SEQ ext number: counter TCPAOBad increased 11 > > => 19 > > ok 17 TCP-AO with wrong receive SEQ ext number: post-migrate connection is > > broken > > ok 18 TCP-AO with wrong receive SEQ ext number: server couldn't serve > > ok 19 TCP-AO with wrong receive SEQ ext number: counter TCPAOBad increased > > 10 => 18 > > ok 20 TCP-AO with wrong receive SEQ ext number: counter TCPAOBad increased > > 20 => 23 > > # Totals: pass:20 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Signed-off-by: Dmitry Safonov Tested-by: Hangbin Liu
Re: [PATCH 09/12] selftests/net: Add SEQ number extension test
On Fri, Dec 15, 2023 at 02:36:23AM +, Dmitry Safonov wrote: > Check that on SEQ number wraparound there is no disruption or TCPAOBad > segments produced. > > Sample of expected output: > > # ./seq-ext_ipv4 > > 1..7 > > # 1436[lib/setup.c:254] rand seed 1686611079 > > TAP version 13 > > ok 1 server alive > > ok 2 post-migrate connection alive > > ok 3 TCPAOGood counter increased 1002 => 3002 > > ok 4 TCPAOGood counter increased 1003 => 3003 > > ok 5 TCPAOBad counter didn't increase > > ok 6 TCPAOBad counter didn't increase > > ok 7 SEQ extension incremented: 1/1999, 1/998999 > > # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Signed-off-by: Dmitry Safonov Tested-by: Hangbin Liu
Re: [PATCH 10/12] selftests/net: Add TCP-AO RST test
On Fri, Dec 15, 2023 at 02:36:24AM +, Dmitry Safonov wrote:
> Check that both active and passive reset works and correctly sign
> segments with TCP-AO or don't send RSTs if not possible to sign.
> A listening socket with backlog = 0 gets one connection in accept
> queue, another in syn queue. Once the server/listener socket is
> forcibly closed, client sockets aren't connected to anything.
> In regular situation they would receive RST on any segment, but
> with TCP-AO as there's no listener, no AO-key and unknown ISNs,
> no RST should be sent.
>
> And "passive" reset, where RST is sent on reply for some segment
> (tcp_v{4,6}_send_reset()) - there use TCP_REPAIR to corrupt SEQ numbers,
> which later results in TCP-AO signed RST, which will be verified and
> client socket will get EPIPE.
>
> No TCPAORequired/TCPAOBad segments are expected during these tests.
>
> Sample of the output:
> > # ./rst_ipv4
> > 1..15
> > # 1462[lib/setup.c:254] rand seed 1686611171
> > TAP version 13
> > ok 1 servered 1000 bytes
> > ok 2 Verified established tcp connection
> > ok 3 sk[0] = 7, connection was reset
> > ok 4 sk[1] = 8, connection was reset
> > ok 5 sk[2] = 9
> > ok 6 MKT counters are good on server
> > ok 7 Verified established tcp connection
> > ok 8 client connection broken post-seq-adjust
> > ok 9 client connection was reset
> > ok 10 No segments without AO sign (server)
> > ok 11 Signed AO segments (server): 0 => 30
> > ok 12 No segments with bad AO sign (server)
> > ok 13 No segments without AO sign (client)
> > ok 14 Signed AO segments (client): 0 => 30
> > ok 15 No segments with bad AO sign (client)
> > # Totals: pass:15 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Signed-off-by: Dmitry Safonov
Tested-by: Hangbin Liu
Re: [PATCH 11/12] selftests/net: Add TCP-AO selfconnect/simultaneous connect test
On Fri, Dec 15, 2023 at 02:36:25AM +, Dmitry Safonov wrote: > Check that a rare functionality of TCP named self-connect works with > TCP-AO. This "under the cover" also checks TCP simultaneous connect > (TCP_SYN_RECV socket state), which would be harder to check other ways. > > In order to verify that it's indeed TCP simultaneous connect, check > the counters TCPChallengeACK and TCPSYNChallenge. > > Sample of the output: > > # ./self-connect_ipv6 > > 1..4 > > # 1738[lib/setup.c:254] rand seed 1696451931 > > TAP version 13 > > ok 1 self-connect(same keyids): connect TCPAOGood 0 => 24 > > ok 2 self-connect(different keyids): connect TCPAOGood 26 => 50 > > ok 3 self-connect(restore): connect TCPAOGood 52 => 97 > > ok 4 self-connect(restore, different keyids): connect TCPAOGood 99 => 144 > > # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:0 error:0 > > Signed-off-by: Dmitry Safonov Tested-by: Hangbin Liu
Re: [PATCH 00/12] selftests/net: Add TCP-AO tests
Hi Dmitry,
I just found the patch set has been merged. Sorry for the noise of
tested-by replies. Please feel free to ignore the question for
unsigned-md5_ipv6. The test passed with 6.7.0-rc5.
Thanks
Hangbin
On Fri, Dec 15, 2023 at 02:36:14AM +, Dmitry Safonov wrote:
> Hi,
>
> An essential part of any big kernel submissions is selftests.
> At the beginning of TCP-AO project, I made patches to fcnal-test.sh
> and nettest.c to have the benefits of easy refactoring, early noticing
> breakages, putting a moat around the code, documenting
> and designing uAPI.
>
> While tests based on fcnal-test.sh/nettest.c provided initial testing*
> and were very easy to add, the pile of TCP-AO quickly grew out of
> one-binary + shell-script testing.
>
> The design of the TCP-AO testing is a bit different than one-big
> selftest binary as I did previously in net/ipsec.c. I found it
> beneficial to avoid implementing a tests runner/scheduler and delegate
> it to the user or Makefile. The approach is very influenced
> by CRIU/ZDTM testing[1]: it provides a static library with helper
> functions and selftest binaries that create specific scenarios.
> I also tried to utilize kselftest.h.
>
> test_init() function does all needed preparations. To not leave
> any traces after a selftest exists, it creates a network namespace
> and if the test wants to establish a TCP connection, a child netns.
> The parent and child netns have veth pair with proper ip addresses
> and routes set up. Both peers, the client and server are different
> pthreads. The treading model was chosen over forking mostly by easiness
> of cleanup on a failure: no need to search for children, handle SIGCHLD,
> make sure not to wait for a dead peer to perform anything, etc.
> Any thread that does exit() naturally kills the tests, sweet!
> The selftests are compiled currently in two variants: ipv4 and ipv6.
> Ipv4-mapped-ipv6 addresses might be a third variant to add, but it's not
> there in this version. As pretty much all tests are shared between two
> address families, most of the code can be shared, too. To differ in code
> what kind of test is running, Makefile supplies -DIPV6_TEST to compiler
> and ifdeffery in tests can do things that have to be different between
> address families. This is similar to TARGETS_C_BOTHBITS in x86 selftests
> and also to tests code sharing in CRIU/ZDTM.
>
> The total number of tests is 832.
> From them rst_ipv{4,6} has currently one flaky subtest, that may fail:
> > not ok 9 client connection was not reset: 0
> I'll investigate what happens there. Also, unsigned-md5_ipv{4,6}
> are flaky because of netns counter checks: it doesn't expect that
> there may be retransmitted TCP segments from a previous sub-selftest.
> That will be fixed. Besides, key-management_ipv{4,6} has 3 sub-tests
> passing with XFAIL:
> > ok 15 # XFAIL listen() after current/rnext keys set: the socket has
> > current/rnext keys: 100:200
> > ok 16 # XFAIL listen socket, delete current key from before listen():
> > failed to delete the key 100:100 -16
> > ok 17 # XFAIL listen socket, delete rnext key from before listen(): failed
> > to delete the key 200:200 -16
> ...
> > # Totals: pass:117 fail:0 xfail:3 xpass:0 skip:0 error:0
> Those need some more kernel work to pass instead of xfail.
>
> The overview of selftests (see the diffstat at the bottom):
> ├── lib
> │ ├── aolib.h
> │ │ The header for all selftests to include.
> │ ├── kconfig.c
> │ │ Kernel kconfig detector to SKIP tests that depend on something.
> │ ├── netlink.c
> │ │ Netlink helper to add/modify/delete VETH/IPs/routes/VRFs
> │ │ I considered just using libmnl, but this is around 400 lines
> │ │ and avoids selftests dependency on out-of-tree sources/packets.
> │ ├── proc.c
> │ │ SNMP/netstat procfs parser and the counters comparator.
> │ ├── repair.c
> │ │ Heavily influenced by libsoccr and reduced to minimum TCP
> │ │ socket checkpoint/repair. Shouldn't be used out of selftests,
> │ │ though.
> │ ├── setup.c
> │ │ All the needed netns/veth/ips/etc preparations for test init.
> │ ├── sock.c
> │ │ Socket helpers: {s,g}etsockopt()s/connect()/listen()/etc.
> │ └── utils.c
> │ Random stuff (a pun intended).
> ├── bench-lookups.c
> │ The only benchmark in selftests currently: checks how well TCP-AO
> │ setsockopt()s perform, depending on the amount of keys on a socket.
> ├── connect.c
> │ Trivial sample, can be used as a boilerplate to write a new test.
> ├── connect-deny.c
> │ More-or-less what could be expected for TCP-AO in fcnal-test.sh
> ├── icmps-accept.c -> icmps-discard.c
> ├── icmps-discard.c
> │ Verifies RFC5925 (7.8) by checking that TCP-AO connection can be
> │ broken if ICMPs are accepted and survives when ::accept_icmps = 0
> ├── key-management.c
> │ Key manipulations, rotations between randomized hashing algorithms
> │ and counter checks for those scenarios.
> ├── restore.c
> │ TCP_AO_REPAIR:
Re: [PATCH v1] selftests/mm: Log run_vmtests.sh results in TAP format
On 16/12/2023 02:40, John Hubbard wrote: > On 12/15/23 18:25, John Hubbard wrote: >> On 12/15/23 06:28, Ryan Roberts wrote: >> ... >>> I've kept all the existing "pretty" output and results summary as is, it >>> just >>> gets a hash in front of it when TAP is enabled. >>> >>> so this: >>> >>> --- >>> running ./hugepage-mmap >>> --- >>> Returned address is 0x89e0 >>> First hex is 0 >>> First hex is 3020100 >>> [PASS] >>> SUMMARY: PASS=1 SKIP=0 FAIL=0 >>> >>> becomes this: >>> >>> TAP version 13 >>> # --- >>> # running ./hugepage-mmap >>> # --- >>> # Returned address is 0x89e0 >>> # First hex is 0 >>> # First hex is 3020100 >>> # [PASS] >>> ok 1 hugepage-mmap >>> # SUMMARY: PASS=1 SKIP=0 FAIL=0 >>> 1..1 >>> >>> If you think the latter is ofensive, then I can do the wrapping as you >>> suggest. >> >> I applied this and ran the tests, all while carefully reminding myself >> to "think like a human". :) And from that perspective, to me, the output >> is effectively the same: the leading '#' characters do not really change >> anything, from a readability point of view. >> >> So IMHO you're on perfectly solid ground, if you just switch over >> directly to this format. Great thanks for taking a look! >> >> Tested-by: John Hubbard >> > > I should also point out that some of the subtests already attempt a TAP > output. So now we end up with TAP-within-TAP output for those programs. It's actually TAP-in-TAP-in-TAP if you're running from run_kselftest.sh :) > > For example: > # --- > # running ./madv_populate > # --- > # TAP version 13 > # 1..21 > # # [RUN] test_prot_read > # ok 1 MADV_POPULATE_READ with PROT_READ > # ok 2 MADV_POPULATE_WRITE with PROT_READ > # # [RUN] test_prot_write > # ok 3 MADV_POPULATE_READ with PROT_WRITE > ...etc... > > Note the double level of leading '#' characters. > > Again, this is still readable enough for humans. But it should probably > be removed in subsequent patches to the subtests. I personally don't agree with this. It would be difficult to flatten to a single TAP instance because the top level doesn't have a clue how many test cases the child is running. Trying to do this will make things more fragile and less modular. LAVA can certainly deal with nested test cases and correctly parses everything to test case names that contain the test name at each level of nesting. The thing I was trying to solve with this patch was that previously the top level (run_kselftest.sh) and the bottom level (individual mm test binaries) were using TAP, but the middle level (run_vmtests.sh) wasn't, and this was confusing the LAVA parser. > > > thanks,
Re: [PATCH] KVM: s390: selftest: memop: Fix undefined behavior
On Fri, 2023-12-15 at 18:02 +0100, Claudio Imbrenda wrote:
> On Fri, 15 Dec 2023 17:11:25 +0100
> Nina Schoetterl-Glausch wrote:
>
> > If an integer's type has x bits, shifting the integer left by x or more
> > is undefined behavior.
> > This can happen in the rotate function when attempting to do a rotation
> > of the whole value by 0.
>
> is 0 the only problematic value? because in that case...
>
> >
> > Fixes: 0dd714bfd200 ("KVM: s390: selftest: memop: Add cmpxchg tests")
> > Signed-off-by: Nina Schoetterl-Glausch
> > ---
> > tools/testing/selftests/kvm/s390x/memop.c | 8 +---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/s390x/memop.c
> > b/tools/testing/selftests/kvm/s390x/memop.c
> > index bb3ca9a5d731..2eba9575828e 100644
> > --- a/tools/testing/selftests/kvm/s390x/memop.c
> > +++ b/tools/testing/selftests/kvm/s390x/memop.c
> > @@ -485,11 +485,13 @@ static bool popcount_eq(__uint128_t a, __uint128_t b)
> >
> > static __uint128_t rotate(int size, __uint128_t val, int amount)
> > {
> > - unsigned int bits = size * 8;
> > + unsigned int left, right, bits = size * 8;
> >
>
> ...why not just:
>
> if (!amount)
> return val;
>
> ?
That works if you move it one statement down (128 would also trigger UB).
% 128 does the trick, is branchless and there is a bit of a symmetry going
on between right and left.
But I can use an early return if you want.
>
> > - amount = (amount + bits) % bits;
> > + right = (amount + bits) % bits;
> > + /* % 128 prevents left shift UB if size == 16 && right == 0 */
> > + left = (bits - right) % 128;
> > val = cut_to_size(size, val);
> > - return (val << (bits - amount)) | (val >> amount);
> > + return (val << left) | (val >> right);
> > }
> >
> > const unsigned int max_block = 16;
> >
> > base-commit: 305230142ae0637213bf6e04f6d9f10bbcb74af8
>
Re: [PATCH net-next v9 02/10] selftests: forwarding: ethtool_mm: fall back to aggregate if device does not report pMAC stats
On 15/12/2023 19:27, Vladimir Oltean wrote:
> On Fri, Dec 15, 2023 at 03:20:40PM +0200, Roger Quadros wrote:
>> diff --git a/tools/testing/selftests/net/forwarding/lib.sh
>> b/tools/testing/selftests/net/forwarding/lib.sh
>> index 8f6ca458af9a..763c262a3453 100755
>> --- a/tools/testing/selftests/net/forwarding/lib.sh
>> +++ b/tools/testing/selftests/net/forwarding/lib.sh
>> @@ -146,6 +146,15 @@ check_ethtool_mm_support()
>> fi
>> }
>>
>> +check_ethtool_pmac_std_stats_support()
>> +{
>> +local dev=$1; shift
>> +local grp=$1; shift
>> +
>> +[ 0 -ne $(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null \
>> +| jq '.[]."$grp" | length') ]
>
> This is broken. $grp inside single quotes will search for the plain-text
> "$grp" string, not for the $grp bash variable. Use ".[].\"$grp\" | length".
>
Thanks for catching this. Will fix in next spin.
> $ ./ethtool_mm.sh eno0 swp0
> eno0 does not report pMAC statistics, falling back to aggregate
> swp0 does not report pMAC statistics, falling back to aggregate
> $ ethtool -S swp0 --all-groups --src pmac
> Standard stats for swp0:
> eth-phy-SymbolErrorDuringCarrier: 0
> eth-mac-FramesTransmittedOK: 90017
> eth-mac-FramesReceivedOK: 90033
> eth-mac-FrameCheckSequenceErrors: 0
> eth-mac-AlignmentErrors: 0
> eth-mac-OctetsTransmittedOK: 9181138
> eth-mac-OctetsReceivedOK: 9182112
> eth-mac-MulticastFramesXmittedOK: 17
> eth-mac-BroadcastFramesXmittedOK: 9
> eth-mac-MulticastFramesReceivedOK: 33
> eth-mac-BroadcastFramesReceivedOK: 9
> eth-mac-FrameTooLongErrors: 0
> eth-ctrl-MACControlFramesReceived: 0
> rmon-etherStatsUndersizePkts: 0
> rmon-etherStatsOversizePkts: 0
> rmon-etherStatsFragments: 0
> rmon-etherStatsJabbers: 0
> rx-rmon-etherStatsPkts64to64Octets: 33
> rx-rmon-etherStatsPkts65to127Octets: 9
> rx-rmon-etherStatsPkts128to255Octets: 0
> rx-rmon-etherStatsPkts256to511Octets: 0
> rx-rmon-etherStatsPkts512to1023Octets: 0
> rx-rmon-etherStatsPkts1024to1526Octets: 0
> rx-rmon-etherStatsPkts1527to65535Octets: 0
> tx-rmon-etherStatsPkts64to64Octets: 12
> tx-rmon-etherStatsPkts65to127Octets: 90005
> tx-rmon-etherStatsPkts128to255Octets: 0
> tx-rmon-etherStatsPkts256to511Octets: 0
> tx-rmon-etherStatsPkts512to1023Octets: 0
> tx-rmon-etherStatsPkts1024to1526Octets: 0
> tx-rmon-etherStatsPkts1527to65535Octets: 0
>
>> +}
>> +
>> check_locked_port_support()
>> {
>> if ! bridge -d link show | grep -q " locked"; then
>> --
>> 2.34.1
>>
--
cheers,
-roger
[PATCH net-next v10 00/10] net: ethernet: am65-cpsw: Add mqprio, frame preemption & coalescing
Hi, This series adds mqprio qdisc offload in channel mode, Frame Preemption MAC merge support and RX/TX coalesing for AM65 CPSW driver. In v10 following changes were made - Fix fallback to aggregate stats check - Added Reviewed-by tags Changelog information in each patch file. cheers, -roger Grygorii Strashko (2): net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in channel mode net: ethernet: ti: am65-cpsw: add sw tx/rx irq coalescing based on hrtimers Roger Quadros (6): net: ethernet: am65-cpsw: Build am65-cpsw-qos only if required net: ethernet: am65-cpsw: Rename TI_AM65_CPSW_TAS to TI_AM65_CPSW_QOS net: ethernet: am65-cpsw: cleanup TAPRIO handling net: ethernet: ti: am65-cpsw: Move code to avoid forward declaration net: ethernet: am65-cpsw: Move register definitions to header file net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support Vladimir Oltean (2): selftests: forwarding: ethtool_mm: support devices with higher rx-min-frag-size selftests: forwarding: ethtool_mm: fall back to aggregate if device does not report pMAC stats drivers/net/ethernet/ti/Kconfig | 14 +- drivers/net/ethernet/ti/Makefile | 3 +- drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 246 ++ drivers/net/ethernet/ti/am65-cpsw-nuss.c | 64 +- drivers/net/ethernet/ti/am65-cpsw-nuss.h | 9 + drivers/net/ethernet/ti/am65-cpsw-qos.c | 708 +- drivers/net/ethernet/ti/am65-cpsw-qos.h | 186 + .../selftests/net/forwarding/ethtool_mm.sh| 48 +- tools/testing/selftests/net/forwarding/lib.sh | 9 + 9 files changed, 1100 insertions(+), 187 deletions(-) base-commit: 610a689d2a57af3e21993cb6d8c3e5f839a8c89e -- 2.34.1
[PATCH net-next v10 01/10] selftests: forwarding: ethtool_mm: support devices with higher rx-min-frag-size
From: Vladimir Oltean
Some devices have errata due to which they cannot report ETH_ZLEN (60)
in the rx-min-frag-size. This was foreseen of course, and lldpad has
logic that when we request it to advertise addFragSize 0, it will round
it up to the lowest value that is _actually_ supported by the hardware.
The problem is that the selftest expects lldpad to report back to us the
same value as we requested.
Make the selftest smarter by figuring out on its own what is a
reasonable value to expect.
Cc: Shuah Khan
Signed-off-by: Vladimir Oltean
Tested-by: Roger Quadros
Signed-off-by: Roger Quadros
---
.../selftests/net/forwarding/ethtool_mm.sh| 37 ++-
1 file changed, 35 insertions(+), 2 deletions(-)
Changelog:
v10: no change.
v9: no code change. Added Kselftest maintainer in Cc.
v8: no change. Moved to the beginning of series.
v7: initial commit
diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
index 39e736f30322..6212913f4ad1 100755
--- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
@@ -155,15 +155,48 @@ manual_failed_verification_h2_to_h1()
manual_failed_verification $h2 $h1
}
+smallest_supported_add_frag_size()
+{
+ local iface=$1
+ local rx_min_frag_size=
+
+ rx_min_frag_size=$(ethtool --json --show-mm $iface | \
+ jq '.[]."rx-min-frag-size"')
+
+ if [ $rx_min_frag_size -le 60 ]; then
+ echo 0
+ elif [ $rx_min_frag_size -le 124 ]; then
+ echo 1
+ elif [ $rx_min_frag_size -le 188 ]; then
+ echo 2
+ elif [ $rx_min_frag_size -le 252 ]; then
+ echo 3
+ else
+ echo "$iface: RX min frag size $rx_min_frag_size cannot be
advertised over LLDP"
+ exit 1
+ fi
+}
+
+expected_add_frag_size()
+{
+ local iface=$1
+ local requested=$2
+ local min=$(smallest_supported_add_frag_size $iface)
+
+ [ $requested -le $min ] && echo $min || echo $requested
+}
+
lldp_change_add_frag_size()
{
local add_frag_size=$1
+ local pattern=
lldptool -T -i $h1 -V addEthCaps addFragSize=$add_frag_size >/dev/null
# Wait for TLVs to be received
sleep 2
- lldptool -i $h2 -t -n -V addEthCaps | \
- grep -q "Additional fragment size: $add_frag_size"
+ pattern=$(printf "Additional fragment size: %d" \
+$(expected_add_frag_size $h1 $add_frag_size))
+ lldptool -i $h2 -t -n -V addEthCaps | grep -q "$pattern"
}
lldp()
--
2.34.1
[PATCH net-next v10 02/10] selftests: forwarding: ethtool_mm: fall back to aggregate if device does not report pMAC stats
From: Vladimir Oltean
Some devices do not support individual 'pmac' and 'emac' stats.
For such devices, resort to 'aggregate' stats.
Cc: Shuah Khan
Signed-off-by: Vladimir Oltean
Tested-by: Roger Quadros
Signed-off-by: Roger Quadros
---
tools/testing/selftests/net/forwarding/ethtool_mm.sh | 11 +++
tools/testing/selftests/net/forwarding/lib.sh| 9 +
2 files changed, 20 insertions(+)
Changelog:
v10: Fix check_ethtool_pmac_std_stats_support() logic.
v9: no code change. Added Kselftest maintainer in Cc.
v8: initial commit
diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
index 6212913f4ad1..50d5bfb17ef1 100755
--- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh
+++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh
@@ -25,6 +25,10 @@ traffic_test()
local after=
local delta=
+ if [ ${has_pmac_stats[$if]} = false ]; then
+ src="aggregate"
+ fi
+
before=$(ethtool_std_stats_get $if "eth-mac" "FramesTransmittedOK" $src)
$MZ $if -q -c $num_pkts -p 64 -b bcast -t ip -R $PREEMPTIBLE_PRIO
@@ -317,6 +321,13 @@ for netif in ${NETIFS[@]}; do
echo "SKIP: $netif does not support MAC Merge"
exit $ksft_skip
fi
+
+ if check_ethtool_pmac_std_stats_support $netif eth-mac; then
+ has_pmac_stats[$netif]=true
+ else
+ has_pmac_stats[$netif]=false
+ echo "$netif does not report pMAC statistics, falling back to
aggregate"
+ fi
done
trap cleanup EXIT
diff --git a/tools/testing/selftests/net/forwarding/lib.sh
b/tools/testing/selftests/net/forwarding/lib.sh
index e3740163c384..69ef2a40df21 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -155,6 +155,15 @@ check_ethtool_counter_group_support()
fi
}
+check_ethtool_pmac_std_stats_support()
+{
+ local dev=$1; shift
+ local grp=$1; shift
+
+ [ 0 -ne $(ethtool --json -S $dev --all-groups --src pmac 2>/dev/null \
+ | jq ".[].\"$grp\" | length") ]
+}
+
check_locked_port_support()
{
if ! bridge -d link show | grep -q " locked"; then
--
2.34.1
[PATCH net-next v10 03/10] net: ethernet: am65-cpsw: Build am65-cpsw-qos only if required
Build am65-cpsw-qos only if CONFIG_TI_AM65_CPSW_TAS is enabled.
Signed-off-by: Roger Quadros
Reviewed-by: Vladimir Oltean
---
drivers/net/ethernet/ti/Makefile| 3 ++-
drivers/net/ethernet/ti/am65-cpsw-qos.c | 12
drivers/net/ethernet/ti/am65-cpsw-qos.h | 26 +
3 files changed, 28 insertions(+), 13 deletions(-)
Changelog:
v10: no code change. Added Reviewed-by tag.
v9: no change
v8: no change
v7: no change
v6: initial commit
diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index 27de1d697134..9d7cd84d1e2d 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -26,7 +26,8 @@ keystone_netcp_ethss-y := netcp_ethss.o netcp_sgmii.o
netcp_xgbepcsr.o cpsw_ale.
obj-$(CONFIG_TI_K3_CPPI_DESC_POOL) += k3-cppi-desc-pool.o
obj-$(CONFIG_TI_K3_AM65_CPSW_NUSS) += ti-am65-cpsw-nuss.o
-ti-am65-cpsw-nuss-y := am65-cpsw-nuss.o cpsw_sl.o am65-cpsw-ethtool.o
cpsw_ale.o am65-cpsw-qos.o
+ti-am65-cpsw-nuss-y := am65-cpsw-nuss.o cpsw_sl.o am65-cpsw-ethtool.o
cpsw_ale.o
+ti-am65-cpsw-nuss-$(CONFIG_TI_AM65_CPSW_TAS) += am65-cpsw-qos.o
ti-am65-cpsw-nuss-$(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV) += am65-cpsw-switchdev.o
obj-$(CONFIG_TI_K3_AM65_CPTS) += am65-cpts.o
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c
b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index 9ac2ff05d501..4bc611cc4aad 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -571,9 +571,6 @@ static int am65_cpsw_setup_taprio(struct net_device *ndev,
void *type_data)
taprio->cmd != TAPRIO_CMD_DESTROY)
return -EOPNOTSUPP;
- if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
- return -ENODEV;
-
if (!netif_running(ndev)) {
dev_err(&ndev->dev, "interface is down, link speed unknown\n");
return -ENETDOWN;
@@ -599,9 +596,6 @@ static int am65_cpsw_tc_query_caps(struct net_device *ndev,
void *type_data)
case TC_SETUP_QDISC_TAPRIO: {
struct tc_taprio_caps *caps = base->caps;
- if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
- return -EOPNOTSUPP;
-
caps->gate_mask_per_txq = true;
return 0;
@@ -806,9 +800,6 @@ void am65_cpsw_qos_link_up(struct net_device *ndev, int
link_speed)
{
struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
- if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
- return;
-
am65_cpsw_est_link_up(ndev, link_speed);
port->qos.link_down_time = 0;
}
@@ -817,9 +808,6 @@ void am65_cpsw_qos_link_down(struct net_device *ndev)
{
struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
- if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
- return;
-
if (!port->qos.link_down_time)
port->qos.link_down_time = ktime_get();
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.h
b/drivers/net/ethernet/ti/am65-cpsw-qos.h
index 0cc2a3b3d7f9..898f13a4a112 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.h
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.h
@@ -31,11 +31,37 @@ struct am65_cpsw_qos {
struct am65_cpsw_ale_ratelimit ale_mc_ratelimit;
};
+#if IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS)
int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type
type,
void *type_data);
void am65_cpsw_qos_link_up(struct net_device *ndev, int link_speed);
void am65_cpsw_qos_link_down(struct net_device *ndev);
int am65_cpsw_qos_ndo_tx_p0_set_maxrate(struct net_device *ndev, int queue,
u32 rate_mbps);
void am65_cpsw_qos_tx_p0_rate_init(struct am65_cpsw_common *common);
+#else
+static inline int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev,
+enum tc_setup_type type,
+void *type_data)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline void am65_cpsw_qos_link_up(struct net_device *ndev,
+int link_speed)
+{ }
+
+static inline void am65_cpsw_qos_link_down(struct net_device *ndev)
+{ }
+
+static inline int am65_cpsw_qos_ndo_tx_p0_set_maxrate(struct net_device *ndev,
+ int queue,
+ u32 rate_mbps)
+{
+ return 0;
+}
+
+static inline void am65_cpsw_qos_tx_p0_rate_init(struct am65_cpsw_common
*common)
+{ }
+#endif
#endif /* AM65_CPSW_QOS_H_ */
--
2.34.1
[PATCH net-next v10 04/10] net: ethernet: am65-cpsw: Rename TI_AM65_CPSW_TAS to TI_AM65_CPSW_QOS
We will use this Kconfig option to not only enable TAS/EST offload
but also other QoS features like Multiqueue priority descriptors
and MAC-Merge/Frame Preemption. TI_AM65_CPSW_QOS seems a more
appropriate Kconfig option name than TI_AM65_CPSW_TAS.
Signed-off-by: Roger Quadros
Reviewed-by: Vladimir Oltean
---
drivers/net/ethernet/ti/Kconfig | 12 ++--
drivers/net/ethernet/ti/Makefile| 2 +-
drivers/net/ethernet/ti/am65-cpsw-qos.h | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
Changelog:
v10: no code change. Added Reviewed-by tag.
v9: no change
v8: initial commit
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index e60b557d59b9..49cd96c4f532 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -134,14 +134,14 @@ config TI_K3_AM65_CPTS
protocol, Ethernet Enhanced Scheduled Traffic Operations (CPTS_ESTFn)
and PCIe Subsystem Precision Time Measurement (PTM).
-config TI_AM65_CPSW_TAS
- bool "Enable TAS offload in AM65 CPSW"
+config TI_AM65_CPSW_QOS
+ bool "Enable QoS offload features in AM65 CPSW"
depends on TI_K3_AM65_CPSW_NUSS && NET_SCH_TAPRIO && TI_K3_AM65_CPTS
help
- Say y here to support Time Aware Shaper(TAS) offload in AM65 CPSW.
- AM65 CPSW hardware supports Enhanced Scheduled Traffic (EST)
- defined in IEEE 802.1Q 2018. The EST scheduler runs on CPTS and the
- TAS/EST schedule is updated in the Fetch RAM memory of the CPSW.
+ This option enables QoS offload features in AM65 CPSW like
+ Time Aware Shaper (TAS) / Enhanced Scheduled Traffic (EST).
+ The EST scheduler runs on CPTS and the TAS/EST schedule is
+ updated in the Fetch RAM memory of the CPSW.
config TI_KEYSTONE_NETCP
tristate "TI Keystone NETCP Core Support"
diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index 9d7cd84d1e2d..d8590304f3df 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -27,7 +27,7 @@ obj-$(CONFIG_TI_K3_CPPI_DESC_POOL) += k3-cppi-desc-pool.o
obj-$(CONFIG_TI_K3_AM65_CPSW_NUSS) += ti-am65-cpsw-nuss.o
ti-am65-cpsw-nuss-y := am65-cpsw-nuss.o cpsw_sl.o am65-cpsw-ethtool.o
cpsw_ale.o
-ti-am65-cpsw-nuss-$(CONFIG_TI_AM65_CPSW_TAS) += am65-cpsw-qos.o
+ti-am65-cpsw-nuss-$(CONFIG_TI_AM65_CPSW_QOS) += am65-cpsw-qos.o
ti-am65-cpsw-nuss-$(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV) += am65-cpsw-switchdev.o
obj-$(CONFIG_TI_K3_AM65_CPTS) += am65-cpts.o
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.h
b/drivers/net/ethernet/ti/am65-cpsw-qos.h
index 898f13a4a112..be4987eb8c51 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.h
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.h
@@ -31,7 +31,7 @@ struct am65_cpsw_qos {
struct am65_cpsw_ale_ratelimit ale_mc_ratelimit;
};
-#if IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS)
+#if IS_ENABLED(CONFIG_TI_AM65_CPSW_QOS)
int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type
type,
void *type_data);
void am65_cpsw_qos_link_up(struct net_device *ndev, int link_speed);
--
2.34.1
[PATCH net-next v10 05/10] net: ethernet: am65-cpsw: cleanup TAPRIO handling
Handle offloading commands using switch-case in
am65_cpsw_setup_taprio().
Move checks to am65_cpsw_taprio_replace().
Use NL_SET_ERR_MSG_MOD for error messages.
Change error message from "Failed to set cycle time extension"
to "cycle time extension not supported"
Signed-off-by: Roger Quadros
Reviewed-by: Vladimir Oltean
---
drivers/net/ethernet/ti/am65-cpsw-qos.c | 151 +++-
1 file changed, 71 insertions(+), 80 deletions(-)
Changelog:
v10: no code change. Added Reviewed-by tag.
v9: no change
v8: don't initialize ret = 0, tact = TACT_PROG
v7: don't use "\n" in NL_SET_ERR_MSG_MOD()
v6: initial commit
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c
b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index 4bc611cc4aad..2c97fa05a852 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -428,7 +428,7 @@ static void am65_cpsw_stop_est(struct net_device *ndev)
am65_cpsw_timer_stop(ndev);
}
-static void am65_cpsw_purge_est(struct net_device *ndev)
+static void am65_cpsw_taprio_destroy(struct net_device *ndev)
{
struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
@@ -441,29 +441,66 @@ static void am65_cpsw_purge_est(struct net_device *ndev)
port->qos.est_admin = NULL;
}
-static int am65_cpsw_configure_taprio(struct net_device *ndev,
- struct am65_cpsw_est *est_new)
+static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from,
+ struct tc_taprio_qopt_offload *to)
+{
+ int i;
+
+ *to = *from;
+ for (i = 0; i < from->num_entries; i++)
+ to->entries[i] = from->entries[i];
+}
+
+static int am65_cpsw_taprio_replace(struct net_device *ndev,
+ struct tc_taprio_qopt_offload *taprio)
{
struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct netlink_ext_ack *extack = taprio->mqprio.extack;
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
struct am65_cpts *cpts = common->cpts;
- int ret = 0, tact = TACT_PROG;
+ struct am65_cpsw_est *est_new;
+ int ret, tact;
- am65_cpsw_est_update_state(ndev);
+ if (!netif_running(ndev)) {
+ NL_SET_ERR_MSG_MOD(extack, "interface is down, link speed
unknown");
+ return -ENETDOWN;
+ }
- if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
- am65_cpsw_stop_est(ndev);
- return ret;
+ if (common->pf_p0_rx_ptype_rrobin) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "p0-rx-ptype-rrobin flag conflicts with
taprio qdisc");
+ return -EINVAL;
+ }
+
+ if (port->qos.link_speed == SPEED_UNKNOWN)
+ return -ENOLINK;
+
+ if (taprio->cycle_time_extension) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "cycle time extension not supported");
+ return -EOPNOTSUPP;
}
+ est_new = devm_kzalloc(&ndev->dev,
+ struct_size(est_new, taprio.entries,
taprio->num_entries),
+ GFP_KERNEL);
+ if (!est_new)
+ return -ENOMEM;
+
+ am65_cpsw_cp_taprio(taprio, &est_new->taprio);
+
+ am65_cpsw_est_update_state(ndev);
+
ret = am65_cpsw_est_check_scheds(ndev, est_new);
if (ret < 0)
- return ret;
+ goto fail;
tact = am65_cpsw_timer_act(ndev, est_new);
if (tact == TACT_NEED_STOP) {
- dev_err(&ndev->dev,
- "Can't toggle estf timer, stop taprio first");
- return -EINVAL;
+ NL_SET_ERR_MSG_MOD(extack,
+ "Can't toggle estf timer, stop taprio
first");
+ ret = -EINVAL;
+ goto fail;
}
if (tact == TACT_PROG)
@@ -476,62 +513,24 @@ static int am65_cpsw_configure_taprio(struct net_device
*ndev,
am65_cpsw_est_set_sched_list(ndev, est_new);
am65_cpsw_port_est_assign_buf_num(ndev, est_new->buf);
- am65_cpsw_est_set(ndev, est_new->taprio.cmd == TAPRIO_CMD_REPLACE);
+ am65_cpsw_est_set(ndev, 1);
if (tact == TACT_PROG) {
ret = am65_cpsw_timer_set(ndev, est_new);
if (ret) {
- dev_err(&ndev->dev, "Failed to set cycle time");
- return ret;
+ NL_SET_ERR_MSG_MOD(extack,
+ "Failed to set cycle time");
+ goto fail;
}
}
- return ret;
-}
-
-static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from,
- struct tc_taprio_qopt_offload *to)
-{
- int i;
-
- *to = *from;
- for (i = 0; i < from->num_entries; i++)
- to->entries[i] = from->entrie
[PATCH net-next v10 06/10] net: ethernet: ti: am65-cpsw: Move code to avoid forward declaration
Move this code around to avoid forward declaration.
No functional change.
Signed-off-by: Roger Quadros
Reviewed-by: Vladimir Oltean
---
drivers/net/ethernet/ti/am65-cpsw-qos.c | 86 -
1 file changed, 43 insertions(+), 43 deletions(-)
v10: no change.
v9: no code change. Added Reviewed-by tag.
v8: no change
v7: move am65_cpsw_qos_tx_rate_calc()
v6: no change
v5: no change
v4: initial commit
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c
b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index 2c97fa05a852..862a98211caa 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -56,6 +56,16 @@ enum timer_act {
TACT_SKIP_PROG, /* just buffer can be updated */
};
+static u32
+am65_cpsw_qos_tx_rate_calc(u32 rate_mbps, unsigned long bus_freq)
+{
+ u32 ir;
+
+ bus_freq /= 100;
+ ir = DIV_ROUND_UP(((u64)rate_mbps * 32768), bus_freq);
+ return ir;
+}
+
static int am65_cpsw_port_est_enabled(struct am65_cpsw_port *port)
{
return port->qos.est_oper || port->qos.est_admin;
@@ -772,49 +782,6 @@ static int am65_cpsw_qos_setup_tc_block(struct net_device
*ndev, struct flow_blo
port, port, true);
}
-int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type
type,
- void *type_data)
-{
- switch (type) {
- case TC_QUERY_CAPS:
- return am65_cpsw_tc_query_caps(ndev, type_data);
- case TC_SETUP_QDISC_TAPRIO:
- return am65_cpsw_setup_taprio(ndev, type_data);
- case TC_SETUP_BLOCK:
- return am65_cpsw_qos_setup_tc_block(ndev, type_data);
- default:
- return -EOPNOTSUPP;
- }
-}
-
-void am65_cpsw_qos_link_up(struct net_device *ndev, int link_speed)
-{
- struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
-
- am65_cpsw_est_link_up(ndev, link_speed);
- port->qos.link_down_time = 0;
-}
-
-void am65_cpsw_qos_link_down(struct net_device *ndev)
-{
- struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
-
- if (!port->qos.link_down_time)
- port->qos.link_down_time = ktime_get();
-
- port->qos.link_speed = SPEED_UNKNOWN;
-}
-
-static u32
-am65_cpsw_qos_tx_rate_calc(u32 rate_mbps, unsigned long bus_freq)
-{
- u32 ir;
-
- bus_freq /= 100;
- ir = DIV_ROUND_UP(((u64)rate_mbps * 32768), bus_freq);
- return ir;
-}
-
static void
am65_cpsw_qos_tx_p0_rate_apply(struct am65_cpsw_common *common,
int tx_ch, u32 rate_mbps)
@@ -916,3 +883,36 @@ void am65_cpsw_qos_tx_p0_rate_init(struct am65_cpsw_common
*common)
host->port_base + AM65_CPSW_PN_REG_PRI_CIR(tx_ch));
}
}
+
+int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type
type,
+ void *type_data)
+{
+ switch (type) {
+ case TC_QUERY_CAPS:
+ return am65_cpsw_tc_query_caps(ndev, type_data);
+ case TC_SETUP_QDISC_TAPRIO:
+ return am65_cpsw_setup_taprio(ndev, type_data);
+ case TC_SETUP_BLOCK:
+ return am65_cpsw_qos_setup_tc_block(ndev, type_data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+void am65_cpsw_qos_link_up(struct net_device *ndev, int link_speed)
+{
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
+
+ am65_cpsw_est_link_up(ndev, link_speed);
+ port->qos.link_down_time = 0;
+}
+
+void am65_cpsw_qos_link_down(struct net_device *ndev)
+{
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
+
+ if (!port->qos.link_down_time)
+ port->qos.link_down_time = ktime_get();
+
+ port->qos.link_speed = SPEED_UNKNOWN;
+}
--
2.34.1
[PATCH net-next v10 07/10] net: ethernet: am65-cpsw: Move register definitions to header file
Move register definitions to header file. No functional change.
Signed-off-by: Roger Quadros
Reviewed-by: Vladimir Oltean
---
drivers/net/ethernet/ti/am65-cpsw-qos.c | 35 -
drivers/net/ethernet/ti/am65-cpsw-qos.h | 35 +
2 files changed, 35 insertions(+), 35 deletions(-)
Changelog:
v10: no change.
v9: no code change. added Reviewed-by tag.
v8: no change
v7: no change
v6: initial commit
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c
b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index 862a98211caa..9f0a05e763d1 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -15,41 +15,6 @@
#include "am65-cpts.h"
#include "cpsw_ale.h"
-#define AM65_CPSW_REG_CTL 0x004
-#define AM65_CPSW_PN_REG_CTL 0x004
-#define AM65_CPSW_PN_REG_FIFO_STATUS 0x050
-#define AM65_CPSW_PN_REG_EST_CTL 0x060
-#define AM65_CPSW_PN_REG_PRI_CIR(pri) (0x140 + 4 * (pri))
-
-/* AM65_CPSW_REG_CTL register fields */
-#define AM65_CPSW_CTL_EST_EN BIT(18)
-
-/* AM65_CPSW_PN_REG_CTL register fields */
-#define AM65_CPSW_PN_CTL_EST_PORT_EN BIT(17)
-
-/* AM65_CPSW_PN_REG_EST_CTL register fields */
-#define AM65_CPSW_PN_EST_ONEBUFBIT(0)
-#define AM65_CPSW_PN_EST_BUFSELBIT(1)
-#define AM65_CPSW_PN_EST_TS_EN BIT(2)
-#define AM65_CPSW_PN_EST_TS_FIRST BIT(3)
-#define AM65_CPSW_PN_EST_ONEPRIBIT(4)
-#define AM65_CPSW_PN_EST_TS_PRI_MSKGENMASK(7, 5)
-
-/* AM65_CPSW_PN_REG_FIFO_STATUS register fields */
-#define AM65_CPSW_PN_FST_TX_PRI_ACTIVE_MSK GENMASK(7, 0)
-#define AM65_CPSW_PN_FST_TX_E_MAC_ALLOW_MSKGENMASK(15, 8)
-#define AM65_CPSW_PN_FST_EST_CNT_ERR BIT(16)
-#define AM65_CPSW_PN_FST_EST_ADD_ERR BIT(17)
-#define AM65_CPSW_PN_FST_EST_BUFACTBIT(18)
-
-/* EST FETCH COMMAND RAM */
-#define AM65_CPSW_FETCH_RAM_CMD_NUM0x80
-#define AM65_CPSW_FETCH_CNT_MSKGENMASK(21, 8)
-#define AM65_CPSW_FETCH_CNT_MAX
(AM65_CPSW_FETCH_CNT_MSK >> 8)
-#define AM65_CPSW_FETCH_CNT_OFFSET 8
-#define AM65_CPSW_FETCH_ALLOW_MSK GENMASK(7, 0)
-#define AM65_CPSW_FETCH_ALLOW_MAX AM65_CPSW_FETCH_ALLOW_MSK
-
enum timer_act {
TACT_PROG, /* need program timer */
TACT_NEED_STOP, /* need stop first */
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.h
b/drivers/net/ethernet/ti/am65-cpsw-qos.h
index be4987eb8c51..e3259f27baad 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.h
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.h
@@ -31,6 +31,41 @@ struct am65_cpsw_qos {
struct am65_cpsw_ale_ratelimit ale_mc_ratelimit;
};
+#define AM65_CPSW_REG_CTL 0x004
+#define AM65_CPSW_PN_REG_CTL 0x004
+#define AM65_CPSW_PN_REG_FIFO_STATUS 0x050
+#define AM65_CPSW_PN_REG_EST_CTL 0x060
+#define AM65_CPSW_PN_REG_PRI_CIR(pri) (0x140 + 4 * (pri))
+
+/* AM65_CPSW_REG_CTL register fields */
+#define AM65_CPSW_CTL_EST_EN BIT(18)
+
+/* AM65_CPSW_PN_REG_CTL register fields */
+#define AM65_CPSW_PN_CTL_EST_PORT_EN BIT(17)
+
+/* AM65_CPSW_PN_REG_EST_CTL register fields */
+#define AM65_CPSW_PN_EST_ONEBUFBIT(0)
+#define AM65_CPSW_PN_EST_BUFSELBIT(1)
+#define AM65_CPSW_PN_EST_TS_EN BIT(2)
+#define AM65_CPSW_PN_EST_TS_FIRST BIT(3)
+#define AM65_CPSW_PN_EST_ONEPRIBIT(4)
+#define AM65_CPSW_PN_EST_TS_PRI_MSKGENMASK(7, 5)
+
+/* AM65_CPSW_PN_REG_FIFO_STATUS register fields */
+#define AM65_CPSW_PN_FST_TX_PRI_ACTIVE_MSK GENMASK(7, 0)
+#define AM65_CPSW_PN_FST_TX_E_MAC_ALLOW_MSKGENMASK(15, 8)
+#define AM65_CPSW_PN_FST_EST_CNT_ERR BIT(16)
+#define AM65_CPSW_PN_FST_EST_ADD_ERR BIT(17)
+#define AM65_CPSW_PN_FST_EST_BUFACTBIT(18)
+
+/* EST FETCH COMMAND RAM */
+#define AM65_CPSW_FETCH_RAM_CMD_NUM0x80
+#define AM65_CPSW_FETCH_CNT_MSKGENMASK(21, 8)
+#define AM65_CPSW_FETCH_CNT_MAX
(AM65_CPSW_FETCH_CNT_MSK >> 8)
+#define AM65_CPSW_FETCH_CNT_OFFSET 8
+#define AM65_CPSW_FETCH_ALLOW_MSK GENMASK(7, 0)
+#define AM65_CPSW_FETCH_ALLOW_MAX AM65_CPSW_FETCH_ALLOW_MSK
+
#if IS_ENABLED(CONFIG_TI_AM65_CPSW_QOS)
int am65_cpsw_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type
type,
void *type_data);
--
2.34.1
[PATCH net-next v10 08/10] net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in channel mode
From: Grygorii Strashko This patch adds MQPRIO Qdisc offload in full 'channel' mode which allows not only setting up pri:tc mapping, but also configuring TX shapers (rate-limiting) on external port FIFOs. The MQPRIO Qdisc offload is expected to work with or without VLAN/priority tagged packets. The CPSW external Port FIFO has 8 Priority queues. The rate-limit can be set for each of these priority queues. Which Priority queue a packet is assigned to depends on PN_REG_TX_PRI_MAP register which maps header priority to switch priority. The header priority of a packet is assigned via the RX_PRI_MAP_REG which maps packet priority to header priority. The packet priority is either the VLAN priority (for VLAN tagged packets) or the thread/channel offset. For simplicity, we assign the same priority queue to all queues of a Traffic Class so it can be rate-limited correctly. Configuration example: ethtool -L eth1 tx 5 ethtool --set-priv-flags eth1 p0-rx-ptype-rrobin off tc qdisc add dev eth1 parent root handle 100: mqprio num_tc 3 \ map 0 0 1 2 0 0 0 0 0 0 0 0 0 0 0 0 \ queues 1@0 1@1 1@2 hw 1 mode channel \ shaper bw_rlimit min_rate 0 100mbit 200mbit max_rate 0 101mbit 202mbit tc qdisc replace dev eth2 handle 100: parent root mqprio num_tc 1 \ map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 queues 1@0 hw 1 ip link add link eth1 name eth1.100 type vlan id 100 ip link set eth1.100 type vlan egress 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7 In the above example two ports share the same TX CPPI queue 0 for low priority traffic. 3 traffic classes are defined for eth1 and mapped to: TC0 - low priority, TX CPPI queue 0 -> ext Port 1 fifo0, no rate limit TC1 - prio 2, TX CPPI queue 1 -> ext Port 1 fifo1, CIR=100Mbit/s, EIR=1Mbit/s TC2 - prio 3, TX CPPI queue 2 -> ext Port 1 fifo2, CIR=200Mbit/s, EIR=2Mbit/s Signed-off-by: Grygorii Strashko Signed-off-by: Roger Quadros Reviewed-by: Vladimir Oltean --- drivers/net/ethernet/ti/Kconfig | 3 +- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 3 + drivers/net/ethernet/ti/am65-cpsw-qos.c | 255 ++- drivers/net/ethernet/ti/am65-cpsw-qos.h | 20 ++ 4 files changed, 277 insertions(+), 4 deletions(-) Changelog v10: - no code change. Added Reviewd-by tag. v9: - drop "\n" from message string passed to NL_SET_ERR_MSG_FMT_MOD() v8: - optimize TO_MBPS() - mention MQPRIO support in TI_AM65_CPSW_QOS Kconfig help text v7: - don't return -EINVAL if num_tc is 0 in setup v6: - fixed typo in comment "packet_priorit"->"packet_priority" - don't call netif_set_real_num_tx_queues() from am65_cpsw_reset_tc_mqprio() - reset TX queue priorities to zero at init. AM65_CPSW_PN_REG_TX_PRI_MAP = 0 - don't ignore return value in am65_cpsw_setup_mqprio() v5: - fix warning at NL_SET_ERR_MSG_FMT_MOD() v4: - use macro TO_MBPS() - use bool for 'shaper_en' - split code movement into separate patch - drop rate vs link speed check as mqprio can be set at link down - call am65_cpsw_reset_tc_mqprio() after pm_runtime_get_sync() v2: - clean up commit message - avoid forward declarations - use tc_mqprio_qopt_offload :: extack for error message reporting - avoid unnecessary mqprio->qopt.hw == 0 checks - set struct tc_mqprio_caps :: validate_queue_counts = true and get rid of am65_cpsw_mqprio_verify() - do not modify AM65_CPSW_P0_REG_RX_PRI_MAP register. diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig index 49cd96c4f532..f76532443298 100644 --- a/drivers/net/ethernet/ti/Kconfig +++ b/drivers/net/ethernet/ti/Kconfig @@ -139,7 +139,8 @@ config TI_AM65_CPSW_QOS depends on TI_K3_AM65_CPSW_NUSS && NET_SCH_TAPRIO && TI_K3_AM65_CPTS help This option enables QoS offload features in AM65 CPSW like - Time Aware Shaper (TAS) / Enhanced Scheduled Traffic (EST). + Time Aware Shaper (TAS) / Enhanced Scheduled Traffic (EST) + and MQPRIO qdisc offload. The EST scheduler runs on CPTS and the TAS/EST schedule is updated in the Fetch RAM memory of the CPSW. diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 7651f90f51f2..2e374745f239 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -2127,6 +2127,9 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common) dev_err(dev, "Use random MAC address\n"); } } + + /* Reset all Queue priorities to 0 */ + writel(0, port->port_base + AM65_CPSW_PN_REG_TX_PRI_MAP); } of_node_put(node); diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c b/drivers/net/ethernet/ti/am65-cpsw-qos.c index 9f0a05e763d1..7ad7af3b3c60 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-qos.c +++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c @@ -7,6 +7,7 @@ */ #include +#include #include #include @@ -15,6 +16,8 @@ #include "am65-cpts.h"
[PATCH net-next v10 09/10] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support
Add driver support for viewing / changing the MAC Merge sublayer
parameters and seeing the verification state machine's current state
via ethtool.
As hardware does not support interrupt notification for verification
events we resort to polling on link up. On link up we try a couple of
times for verification success and if unsuccessful then give up.
The Frame Preemption feature is described in the Technical Reference
Manual [1] in section:
12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0)
Due to Silicon Errata i2208 [2] we set limit min IET fragment size to
124 (excluding 4 bytes mCRC).
[1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
[2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf
Signed-off-by: Roger Quadros
Reviewed-by: Vladimir Oltean
---
drivers/net/ethernet/ti/Kconfig | 5 +-
drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 167 +++
drivers/net/ethernet/ti/am65-cpsw-nuss.c| 2 +
drivers/net/ethernet/ti/am65-cpsw-nuss.h| 5 +
drivers/net/ethernet/ti/am65-cpsw-qos.c | 175
drivers/net/ethernet/ti/am65-cpsw-qos.h | 105
6 files changed, 457 insertions(+), 2 deletions(-)
Changelog:
v10:
- no change.
v9:
- fix up am65_cpsw_get_eth_mac_stats() by merging patch
"net: ethernet: ti: am65-cpsw: Fix get_eth_mac_stats"
- remove unnecessary 'if (cfg->verify_time > 0)' check in am65_cpsw_set_mm()
v8:
- fix build if CONFIG_TI_AM65_CPSW_TAS not enabled
- change CONFIG_TI_AM65_CPSW_TAS to CONFIG_TI_AM65_CPSW_QOS
- mention about Frame Preemption support in TI_AM65_CPSW_QOS Kconfig help
- mentioned IET fragment size excludes 4 bytes mCRC in commit log
v7:
- use else if
- drop FIXME comment
- fix lldp kselftest failure by limiting max_verify_time to spec limit of 128ms.
- now passes all ethtool_mm.sh kselftests (patch 8 required)
v6:
- get mutex around am65_cpsw_iet_commit_preemptible_tcs() in
am65_cpsw_iet_change_preemptible_tcs()
- use "preemption" instead of "pre-emption"
- call am65_cpsw_setup_mqprio() from within am65_cpsw_setup_taprio()
- Now works with kselftest except the last test which fails
v5:
- No change
v4:
- Rebase and include in the same series as mqprio support.
v3:
- Rebase on top of v6.6-rc1 and mqprio support [1]
- Support ethtool_ops :: get_mm_stats()
- drop unused variables cmn_ctrl and verify_cnt
- make am65_cpsw_iet_link_state_update() and
am65_cpsw_iet_change_preemptible_tcs() static
[1] https://lore.kernel.org/all/20230918075358.5878-1-rog...@kernel.org/
v2:
- Use proper control bits for PMAC enable (AM65_CPSW_PN_CTL_IET_PORT_EN)
and TX enable (AM65_CPSW_PN_IET_MAC_PENABLE)
- Common IET Enable (AM65_CPSW_CTL_IET_EN) is set if any port has
AM65_CPSW_PN_CTL_IET_PORT_EN set.
- Fix workaround for erratum i2208. i.e. Limit rx_min_frag_size to 124
- Fix am65_cpsw_iet_get_verify_timeout_ms() to default to timeout for
1G link if link is inactive.
- resize the RX FIFO based on pmac_enabled, not tx_enabled.
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index f76532443298..be01450c20dc 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -139,8 +139,9 @@ config TI_AM65_CPSW_QOS
depends on TI_K3_AM65_CPSW_NUSS && NET_SCH_TAPRIO && TI_K3_AM65_CPTS
help
This option enables QoS offload features in AM65 CPSW like
- Time Aware Shaper (TAS) / Enhanced Scheduled Traffic (EST)
- and MQPRIO qdisc offload.
+ Time Aware Shaper (TAS) / Enhanced Scheduled Traffic (EST),
+ MQPRIO qdisc offload and Frame-Preemption MAC Merge / Interspersing
+ Express Traffic (IET).
The EST scheduler runs on CPTS and the TAS/EST schedule is
updated in the Fetch RAM memory of the CPSW.
diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
index b9e1d568604b..0a6134cdf39f 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
@@ -11,6 +11,7 @@
#include
#include "am65-cpsw-nuss.h"
+#include "am65-cpsw-qos.h"
#include "cpsw_ale.h"
#include "am65-cpts.h"
@@ -670,6 +671,9 @@ static void am65_cpsw_get_eth_mac_stats(struct net_device
*ndev,
stats = port->stat_base;
+ if (s->src != ETHTOOL_MAC_STATS_SRC_AGGREGATE)
+ return;
+
s->FramesTransmittedOK = readl_relaxed(&stats->tx_good_frames);
s->SingleCollisionFrames = readl_relaxed(&stats->tx_single_coll_frames);
s->MultipleCollisionFrames = readl_relaxed(&stats->tx_mult_coll_frames);
@@ -740,6 +744,166 @@ static int am65_cpsw_set_ethtool_priv_flags(struct
net_device *ndev, u32 flags)
return 0;
}
+static void am65_cpsw_port_iet_rx_enable(struct am65_cpsw_port *port, bool
enable)
+{
+ u32 val;
+
+ val = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
+ if (enable)
+ val
[PATCH net-next v10 10/10] net: ethernet: ti: am65-cpsw: add sw tx/rx irq coalescing based on hrtimers
From: Grygorii Strashko
Add SW IRQ coalescing based on hrtimers for TX and RX data path which
can be enabled by ethtool commands:
- RX coalescing
ethtool -C eth1 rx-usecs 50
- TX coalescing can be enabled per TX queue
- by default enables coalesing for TX0
ethtool -C eth1 tx-usecs 50
- configure TX0
ethtool -Q eth0 queue_mask 1 --coalesce tx-usecs 100
- configure TX1
ethtool -Q eth0 queue_mask 2 --coalesce tx-usecs 100
- configure TX0 and TX1
ethtool -Q eth0 queue_mask 3 --coalesce tx-usecs 100 --coalesce tx-usecs 100
show configuration for TX0 and TX1:
ethtool -Q eth0 queue_mask 3 --show-coalesce
Comparing to gro_flush_timeout and napi_defer_hard_irqs, this patch
allows to enable IRQ coalesing for RX path separately.
Signed-off-by: Grygorii Strashko
Signed-off-by: Roger Quadros
---
drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 79 +
drivers/net/ethernet/ti/am65-cpsw-nuss.c| 59 ---
drivers/net/ethernet/ti/am65-cpsw-nuss.h| 4 ++
3 files changed, 134 insertions(+), 8 deletions(-)
Changelog:
v10: no change
v9: no change
v8: no change
v7: no change
v6: no change
v5: initial commit
diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
index 0a6134cdf39f..35fceba01ea4 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
@@ -904,6 +904,80 @@ static void am65_cpsw_get_mm_stats(struct net_device *ndev,
s->MACMergeHoldCount = readl(base + AM65_CPSW_STATN_IET_TX_HOLD);
}
+static int am65_cpsw_get_coalesce(struct net_device *ndev, struct
ethtool_coalesce *coal,
+ struct kernel_ethtool_coalesce *kernel_coal,
+ struct netlink_ext_ack *extack)
+{
+ struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct am65_cpsw_tx_chn *tx_chn;
+
+ tx_chn = &common->tx_chns[0];
+
+ coal->rx_coalesce_usecs = common->rx_pace_timeout / 1000;
+ coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout / 1000;
+
+ return 0;
+}
+
+static int am65_cpsw_get_per_queue_coalesce(struct net_device *ndev, u32 queue,
+ struct ethtool_coalesce *coal)
+{
+ struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct am65_cpsw_tx_chn *tx_chn;
+
+ if (queue >= AM65_CPSW_MAX_TX_QUEUES)
+ return -EINVAL;
+
+ tx_chn = &common->tx_chns[queue];
+
+ coal->tx_coalesce_usecs = tx_chn->tx_pace_timeout / 1000;
+
+ return 0;
+}
+
+static int am65_cpsw_set_coalesce(struct net_device *ndev, struct
ethtool_coalesce *coal,
+ struct kernel_ethtool_coalesce *kernel_coal,
+ struct netlink_ext_ack *extack)
+{
+ struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct am65_cpsw_tx_chn *tx_chn;
+
+ tx_chn = &common->tx_chns[0];
+
+ if (coal->rx_coalesce_usecs && coal->rx_coalesce_usecs < 20)
+ return -EINVAL;
+
+ if (coal->tx_coalesce_usecs && coal->tx_coalesce_usecs < 20)
+ return -EINVAL;
+
+ common->rx_pace_timeout = coal->rx_coalesce_usecs * 1000;
+ tx_chn->tx_pace_timeout = coal->tx_coalesce_usecs * 1000;
+
+ return 0;
+}
+
+static int am65_cpsw_set_per_queue_coalesce(struct net_device *ndev, u32 queue,
+ struct ethtool_coalesce *coal)
+{
+ struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct am65_cpsw_tx_chn *tx_chn;
+
+ if (queue >= AM65_CPSW_MAX_TX_QUEUES)
+ return -EINVAL;
+
+ tx_chn = &common->tx_chns[queue];
+
+ if (coal->tx_coalesce_usecs && coal->tx_coalesce_usecs < 20) {
+ dev_info(common->dev, "defaulting to min value of 20us for
tx-usecs for tx-%u\n",
+queue);
+ coal->tx_coalesce_usecs = 20;
+ }
+
+ tx_chn->tx_pace_timeout = coal->tx_coalesce_usecs * 1000;
+
+ return 0;
+}
+
const struct ethtool_ops am65_cpsw_ethtool_ops_slave = {
.begin = am65_cpsw_ethtool_op_begin,
.complete = am65_cpsw_ethtool_op_complete,
@@ -922,6 +996,11 @@ const struct ethtool_ops am65_cpsw_ethtool_ops_slave = {
.get_ts_info= am65_cpsw_get_ethtool_ts_info,
.get_priv_flags = am65_cpsw_get_ethtool_priv_flags,
.set_priv_flags = am65_cpsw_set_ethtool_priv_flags,
+ .supported_coalesce_params = ETHTOOL_COALESCE_USECS,
+ .get_coalesce = am65_cpsw_get_coalesce,
+ .set_coalesce = am65_cpsw_set_coalesce,
+ .get_per_queue_coalesce = am65_cpsw_get_per_queue_coalesce,
+ .set_per_queue_coalesce = am65_cpsw_set_per_queue_coalesce,
.get_link = ethtool_op_get_link,
.get_link_ksettings = am65_cpsw_get_lin
Re: [PATCH net-next v9 08/10] net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in channel mode
On Fri, Dec 15, 2023 at 03:20:46PM +0200, Roger Quadros wrote: > From: Grygorii Strashko > > This patch adds MQPRIO Qdisc offload in full 'channel' mode which allows > not only setting up pri:tc mapping, but also configuring TX shapers > (rate-limiting) on external port FIFOs. > > The MQPRIO Qdisc offload is expected to work with or without VLAN/priority > tagged packets. > > The CPSW external Port FIFO has 8 Priority queues. The rate-limit can be > set for each of these priority queues. Which Priority queue a packet is > assigned to depends on PN_REG_TX_PRI_MAP register which maps header > priority to switch priority. > > The header priority of a packet is assigned via the RX_PRI_MAP_REG which > maps packet priority to header priority. > > The packet priority is either the VLAN priority (for VLAN tagged packets) > or the thread/channel offset. > > For simplicity, we assign the same priority queue to all queues of a > Traffic Class so it can be rate-limited correctly. > > Configuration example: > ethtool -L eth1 tx 5 > ethtool --set-priv-flags eth1 p0-rx-ptype-rrobin off > > tc qdisc add dev eth1 parent root handle 100: mqprio num_tc 3 \ > map 0 0 1 2 0 0 0 0 0 0 0 0 0 0 0 0 \ > queues 1@0 1@1 1@2 hw 1 mode channel \ > shaper bw_rlimit min_rate 0 100mbit 200mbit max_rate 0 101mbit 202mbit > > tc qdisc replace dev eth2 handle 100: parent root mqprio num_tc 1 \ > map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 queues 1@0 hw 1 > > ip link add link eth1 name eth1.100 type vlan id 100 > ip link set eth1.100 type vlan egress 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7 > > In the above example two ports share the same TX CPPI queue 0 for low > priority traffic. 3 traffic classes are defined for eth1 and mapped to: > TC0 - low priority, TX CPPI queue 0 -> ext Port 1 fifo0, no rate limit > TC1 - prio 2, TX CPPI queue 1 -> ext Port 1 fifo1, CIR=100Mbit/s, EIR=1Mbit/s > TC2 - prio 3, TX CPPI queue 2 -> ext Port 1 fifo2, CIR=200Mbit/s, EIR=2Mbit/s > > Signed-off-by: Grygorii Strashko > Signed-off-by: Roger Quadros ... > diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c > b/drivers/net/ethernet/ti/am65-cpsw-qos.c > index 9f0a05e763d1..7ad7af3b3c60 100644 > --- a/drivers/net/ethernet/ti/am65-cpsw-qos.c > +++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c > @@ -7,6 +7,7 @@ > */ > > #include > +#include > #include > #include > > @@ -15,6 +16,8 @@ > #include "am65-cpts.h" > #include "cpsw_ale.h" > > +#define TO_MBPS(x) DIV_ROUND_UP((x), BYTES_PER_MBIT) Hi Grygorii and Roger, a minor nit from my side: in order for BYTES_PER_MBIT to be defined linux/units.h needs to be included. But that isn't added until the next patch. ...
[PATCH][next] selftests/net: Fix various spelling mistakes in TCP-AO tests
There are a handful of spelling mistakes in test messages in the
TCP-AIO selftests. Fix these.
Signed-off-by: Colin Ian King
---
tools/testing/selftests/net/tcp_ao/connect-deny.c | 2 +-
tools/testing/selftests/net/tcp_ao/lib/proc.c | 4 ++--
tools/testing/selftests/net/tcp_ao/setsockopt-closed.c | 2 +-
tools/testing/selftests/net/tcp_ao/unsigned-md5.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/net/tcp_ao/connect-deny.c
b/tools/testing/selftests/net/tcp_ao/connect-deny.c
index 1ca78040d8b7..185a2f6e5ff3 100644
--- a/tools/testing/selftests/net/tcp_ao/connect-deny.c
+++ b/tools/testing/selftests/net/tcp_ao/connect-deny.c
@@ -55,7 +55,7 @@ static void try_accept(const char *tst_name, unsigned int
port, const char *pwd,
err = test_wait_fd(lsk, timeout, 0);
if (err == -ETIMEDOUT) {
if (!fault(TIMEOUT))
- test_fail("timeouted for accept()");
+ test_fail("timed out for accept()");
} else if (err < 0) {
test_error("test_wait_fd()");
} else {
diff --git a/tools/testing/selftests/net/tcp_ao/lib/proc.c
b/tools/testing/selftests/net/tcp_ao/lib/proc.c
index 2322f4d4676d..2fb6dd8adba6 100644
--- a/tools/testing/selftests/net/tcp_ao/lib/proc.c
+++ b/tools/testing/selftests/net/tcp_ao/lib/proc.c
@@ -227,7 +227,7 @@ void netstat_print_diff(struct netstat *nsa, struct netstat
*nsb)
}
if (nsb->counters_nr < nsa->counters_nr)
- test_error("Unexpected: some counters dissapeared!");
+ test_error("Unexpected: some counters disappeared!");
for (j = 0, i = 0; i < nsb->counters_nr; i++) {
if (strcmp(nsb->counters[i].name,
nsa->counters[j].name)) {
@@ -244,7 +244,7 @@ void netstat_print_diff(struct netstat *nsa, struct netstat
*nsb)
j++;
}
if (j != nsa->counters_nr)
- test_error("Unexpected: some counters dissapeared!");
+ test_error("Unexpected: some counters disappeared!");
nsb = nsb->next;
nsa = nsa->next;
diff --git a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
index 7e4601b3f6a3..a329f42f40ce 100644
--- a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
+++ b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
@@ -427,7 +427,7 @@ static void test_einval_del_key(void)
sk = prepare_defs(TCP_AO_DEL_KEY, &del);
del.set_current = 1;
- setsockopt_checked(sk, TCP_AO_DEL_KEY, &del, ENOENT, "set non-exising
current key");
+ setsockopt_checked(sk, TCP_AO_DEL_KEY, &del, ENOENT, "set non-existing
current key");
sk = prepare_defs(TCP_AO_DEL_KEY, &del);
del.set_rnext = 1;
diff --git a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
index 7cffde02d2be..14addfd46468 100644
--- a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
+++ b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
@@ -72,7 +72,7 @@ static void try_accept(const char *tst_name, unsigned int
port,
err = test_wait_fd(lsk, timeout, 0);
if (err == -ETIMEDOUT) {
if (!fault(TIMEOUT))
- test_fail("timeouted for accept()");
+ test_fail("timed out for accept()");
} else if (err < 0) {
test_error("test_wait_fd()");
} else {
--
2.39.2
Re: [PATCH] KVM: s390: selftest: memop: Fix undefined behavior
On Mon, 18 Dec 2023 13:18:14 +0100
Nina Schoetterl-Glausch wrote:
[...]
> > > diff --git a/tools/testing/selftests/kvm/s390x/memop.c
> > > b/tools/testing/selftests/kvm/s390x/memop.c
> > > index bb3ca9a5d731..2eba9575828e 100644
> > > --- a/tools/testing/selftests/kvm/s390x/memop.c
> > > +++ b/tools/testing/selftests/kvm/s390x/memop.c
> > > @@ -485,11 +485,13 @@ static bool popcount_eq(__uint128_t a, __uint128_t
> > > b)
> > >
> > > static __uint128_t rotate(int size, __uint128_t val, int amount)
> > > {
> > > - unsigned int bits = size * 8;
> > > + unsigned int left, right, bits = size * 8;
> > >
> >
> > ...why not just:
> >
> > if (!amount)
> > return val;
> >
> > ?
>
> That works if you move it one statement down (128 would also trigger UB).
oops, yes it has to be after
> % 128 does the trick, is branchless and there is a bit of a symmetry going
> on between right and left.
> But I can use an early return if you want.
I think it's more readable, and furthermore...
>
> >
> > > - amount = (amount + bits) % bits;
> > > + right = (amount + bits) % bits;
> > > + /* % 128 prevents left shift UB if size == 16 && right == 0 */
> > > + left = (bits - right) % 128;
> > > val = cut_to_size(size, val);
> > > - return (val << (bits - amount)) | (val >> amount);
...this is a more idiomatic syntax for a rotate operation
> > > + return (val << left) | (val >> right);
> > > }
> > >
> > > const unsigned int max_block = 16;
> > >
> > > base-commit: 305230142ae0637213bf6e04f6d9f10bbcb74af8
> >
>
[PATCH] kunit: Fix NULL-dereference in kunit_init_suite() if suite->log is NULL
suite->log must be checked for NULL before passing it to
string_stream_clear(). This was done in kunit_init_test() but was missing
from kunit_init_suite().
Signed-off-by: Richard Fitzgerald
Fixes: 6d696c4695c5 ("kunit: add ability to run tests after boot using debugfs")
---
lib/kunit/test.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index e803d998e855..ea7f0913e55a 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -658,7 +658,9 @@ static void kunit_init_suite(struct kunit_suite *suite)
kunit_debugfs_create_suite(suite);
suite->status_comment[0] = '\0';
suite->suite_init_err = 0;
- string_stream_clear(suite->log);
+
+ if (suite->log)
+ string_stream_clear(suite->log);
}
bool kunit_enabled(void)
--
2.30.2
Re: [PATCH net-next v9 08/10] net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in channel mode
Hi Simon, On 18/12/2023 15:43, Simon Horman wrote: > On Fri, Dec 15, 2023 at 03:20:46PM +0200, Roger Quadros wrote: >> From: Grygorii Strashko >> >> This patch adds MQPRIO Qdisc offload in full 'channel' mode which allows >> not only setting up pri:tc mapping, but also configuring TX shapers >> (rate-limiting) on external port FIFOs. >> >> The MQPRIO Qdisc offload is expected to work with or without VLAN/priority >> tagged packets. >> >> The CPSW external Port FIFO has 8 Priority queues. The rate-limit can be >> set for each of these priority queues. Which Priority queue a packet is >> assigned to depends on PN_REG_TX_PRI_MAP register which maps header >> priority to switch priority. >> >> The header priority of a packet is assigned via the RX_PRI_MAP_REG which >> maps packet priority to header priority. >> >> The packet priority is either the VLAN priority (for VLAN tagged packets) >> or the thread/channel offset. >> >> For simplicity, we assign the same priority queue to all queues of a >> Traffic Class so it can be rate-limited correctly. >> >> Configuration example: >> ethtool -L eth1 tx 5 >> ethtool --set-priv-flags eth1 p0-rx-ptype-rrobin off >> >> tc qdisc add dev eth1 parent root handle 100: mqprio num_tc 3 \ >> map 0 0 1 2 0 0 0 0 0 0 0 0 0 0 0 0 \ >> queues 1@0 1@1 1@2 hw 1 mode channel \ >> shaper bw_rlimit min_rate 0 100mbit 200mbit max_rate 0 101mbit 202mbit >> >> tc qdisc replace dev eth2 handle 100: parent root mqprio num_tc 1 \ >> map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 queues 1@0 hw 1 >> >> ip link add link eth1 name eth1.100 type vlan id 100 >> ip link set eth1.100 type vlan egress 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7 >> >> In the above example two ports share the same TX CPPI queue 0 for low >> priority traffic. 3 traffic classes are defined for eth1 and mapped to: >> TC0 - low priority, TX CPPI queue 0 -> ext Port 1 fifo0, no rate limit >> TC1 - prio 2, TX CPPI queue 1 -> ext Port 1 fifo1, CIR=100Mbit/s, EIR=1Mbit/s >> TC2 - prio 3, TX CPPI queue 2 -> ext Port 1 fifo2, CIR=200Mbit/s, EIR=2Mbit/s >> >> Signed-off-by: Grygorii Strashko >> Signed-off-by: Roger Quadros > > ... > >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c >> b/drivers/net/ethernet/ti/am65-cpsw-qos.c >> index 9f0a05e763d1..7ad7af3b3c60 100644 >> --- a/drivers/net/ethernet/ti/am65-cpsw-qos.c >> +++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c >> @@ -7,6 +7,7 @@ >> */ >> >> #include >> +#include >> #include >> #include >> >> @@ -15,6 +16,8 @@ >> #include "am65-cpts.h" >> #include "cpsw_ale.h" >> >> +#define TO_MBPS(x) DIV_ROUND_UP((x), BYTES_PER_MBIT) > > Hi Grygorii and Roger, > > a minor nit from my side: in order for BYTES_PER_MBIT to be defined > linux/units.h needs to be included. But that isn't added until > the next patch. Thanks for the catch. I'll fix it in next spin. -- cheers, -roger
[PATCH 1/2] kunit: Allow passing function pointer to kunit_activate_static_stub()
Swap the arguments to typecheck_fn() in kunit_activate_static_stub()
so that real_fn_addr can be either the function itself or a pointer
to that function.
This is useful to simplify redirecting static functions in a module.
Having to pass the actual function meant that it must be exported
from the module. Either making the 'static' and EXPORT_SYMBOL*()
conditional (which makes the code messy), or change it to always
exported (which increases the export namespace and prevents the
compiler inlining a trivial stub function in non-test builds).
With the original definition of kunit_activate_static_stub() the
address of real_fn_addr was passed to typecheck_fn() as the type to
be passed. This meant that if real_fn_addr was a pointer-to-function
it would resolve to a ** instead of a *, giving an error like this:
error: initialization of ‘int (**)(int)’ from incompatible pointer
type ‘int (*)(int)’ [-Werror=incompatible-pointer-types]
kunit_activate_static_stub(test, add_one_fn_ptr, subtract_one);
| ^~~~
./include/linux/typecheck.h:21:25: note: in definition of macro
‘typecheck_fn’
21 | ({ typeof(type) __tmp = function; \
Swapping the arguments to typecheck_fn makes it take the type of a
pointer to the replacement function. Either a function or a pointer
to function can be assigned to that. For example:
static int some_function(int x)
{
/* whatever */
}
int (* some_function_ptr)(int) = some_function;
static int replacement(int x)
{
/* whatever */
}
Then:
kunit_activate_static_stub(test, some_function, replacement);
yields:
typecheck_fn(typeof(&replacement), some_function);
and:
kunit_activate_static_stub(test, some_function_ptr, replacement);
yields:
typecheck_fn(typeof(&replacement), some_function_ptr);
The two typecheck_fn() then resolve to:
int (*__tmp)(int) = some_function;
and
int (*__tmp)(int) = some_function_ptr;
Both of these are valid. In the first case the compiler inserts
an implicit '&' to take the address of the supplied function, and
in the second case the RHS is already a pointer to the same type.
Signed-off-by: Richard Fitzgerald
---
include/kunit/static_stub.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h
index 85315c80b303..bf940322dfc0 100644
--- a/include/kunit/static_stub.h
+++ b/include/kunit/static_stub.h
@@ -93,7 +93,7 @@ void __kunit_activate_static_stub(struct kunit *test,
* The redirection can be disabled again with kunit_deactivate_static_stub().
*/
#define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do {
\
- typecheck_fn(typeof(&real_fn_addr), replacement_addr);
\
+ typecheck_fn(typeof(&replacement_addr), real_fn_addr);
\
__kunit_activate_static_stub(test, real_fn_addr, replacement_addr);
\
} while (0)
--
2.30.2
[PATCH 2/2] kunit: Add example of kunit_activate_static_stub() with pointer-to-function
Adds a variant of example_static_stub_test() that shows use of a
pointer-to-function with kunit_activate_static_stub().
A const pointer to the add_one() function is declared. This
pointer-to-function is passed to kunit_activate_static_stub() and
kunit_deactivate_static_stub() instead of passing add_one directly.
Signed-off-by: Richard Fitzgerald
---
lib/kunit/kunit-example-test.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index d2f7a3c62c18..9e57f341dc37 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -168,6 +168,8 @@ static int subtract_one(int i)
return i - 1;
}
+static int (* const add_one_fn_ptr)(int i) = add_one;
+
/*
* This test shows the use of static stubs.
*/
@@ -187,6 +189,30 @@ static void example_static_stub_test(struct kunit *test)
KUNIT_EXPECT_EQ(test, add_one(1), 2);
}
+/*
+ * This test shows the use of static stubs when the function being
+ * replaced is provided as a pointer-to-function instead of the
+ * actual function. This is useful for providing access to static
+ * functions in a module by exporting a pointer to that function
+ * instead of having to change the static function to a non-static
+ * exported function.
+ */
+static void example_static_stub_using_fn_ptr_test(struct kunit *test)
+{
+ /* By default, function is not stubbed. */
+ KUNIT_EXPECT_EQ(test, add_one(1), 2);
+
+ /* Replace add_one() with subtract_one(). */
+ kunit_activate_static_stub(test, add_one_fn_ptr, subtract_one);
+
+ /* add_one() is now replaced. */
+ KUNIT_EXPECT_EQ(test, add_one(1), 0);
+
+ /* Return add_one() to normal. */
+ kunit_deactivate_static_stub(test, add_one_fn_ptr);
+ KUNIT_EXPECT_EQ(test, add_one(1), 2);
+}
+
static const struct example_param {
int value;
} example_params_array[] = {
@@ -245,6 +271,7 @@ static struct kunit_case example_test_cases[] = {
KUNIT_CASE(example_mark_skipped_test),
KUNIT_CASE(example_all_expect_macros_test),
KUNIT_CASE(example_static_stub_test),
+ KUNIT_CASE(example_static_stub_using_fn_ptr_test),
KUNIT_CASE_PARAM(example_params_test, example_gen_params),
KUNIT_CASE_SLOW(example_slow_test),
{}
--
2.30.2
Re: selftests/livepatch fails on s390
On 12/18/23 05:44, Alexander Gordeev wrote:
> Hi all,
>
> The livepatch selftest somehow fails in -next on s390 due to what
> appears to me as 'comm' usage issue. E.g the removal of timestamp-
> less line "with link type OSD_10GIG." in the below output forces
> 'comm' to produce the correct result in check_result() function of
> tools/testing/selftests/livepatch/functions.sh script:
>
> [ 11.229256] qeth 0.0.bd02: qdio: OSA on SC 2624 using AI:1 QEBSM:0 PRI:1
> TDD:1 SIGA: W
> [ 11.250189] systemd-journald[943]: Successfully sent stream file
> descriptor to service manager.
> [ 11.258763] qeth 0.0.bd00: Device is a OSD Express card (level: 0165)
>with link type OSD_10GIG.
> [ 11.259261] qeth 0.0.bd00: The device represents a Bridge Capable Port
> [ 11.262376] qeth 0.0.bd00: MAC address b2:96:9c:49:aa:e9 successfully
> registered
> [ 11.269654] qeth 0.0.bd00: MAC address 06:c6:b5:7d:ee:63 successfully
> registered
>
> By contrast, using the 'diff' instead works as a charm. But it was
> removed with commit 2f3f651f3756 ("selftests/livepatch: Use "comm"
> instead of "diff" for dmesg").
>
> I am attaching the contents of "$expect" and "$result" script
> variables and the output of 'dmesg' before and after test run
> dmesg-saved.txt and dmesg.txt.
>
> Another 'dmesg' output dmesg-saved1.txt and dmesg1.txt also
> shows the same problem, which seems like something to do with
> sorting.
>
> The minimal reproducer attached is dmesg-saved1-rep.txt and
> dmesg1-rep.txt, that could be described as:
>
> --- dmesg-saved1-rep.txt 2023-12-17 21:08:14.171014218 +0100
> +++ dmesg1-rep.txt2023-12-17 21:06:52.221014218 +0100
> @@ -1,3 +1,3 @@
> -[ 98.820331] livepatch: 'test_klp_state2': starting patching transition
> [ 100.031067] livepatch: 'test_klp_state2': completing patching transition
> [ 284.224335] livepatch: kernel.ftrace_enabled = 1
> +[ 284.232921] = TEST: basic shadow variable API =
>
> The culprit is the extra space in [ 98.820331] timestamp, that from
> the script point of view produces the output with two extra lines:
>
> [ 100.031067] livepatch: 'test_klp_state2': completing patching transition
> [ 284.224335] livepatch: kernel.ftrace_enabled = 1
> [ 284.232921] = TEST: basic shadow variable API =
>
> If the line with [ 98.820331] timestamp removed or changed to e.g
> [ 100.031066] (aka 1 us less), then the result output is as expected:
>
> [ 284.232921] = TEST: basic shadow variable API =
>
> Thanks!
Hi Alexander,
You're right about the timestamp formatting. I can repro with a a
slight variant on your example:
Assume a pre-test dmesg log has three msgs (including one with timestamp
of fewer digits):
$ head /tmp/{A,B}
==> /tmp/A <==
[ 1] message one
[10] message two
[11] message three
during the rest, the first message rolls off the buffer post-test and a
new fourth message is added:
==> /tmp/B <==
[10] message two
[11] message three
[12] message four
The test's comm invocation should be only printing "lines unique to
FILE2", ie, the latest fourth message, but...
$ comm --nocheck-order -13 /tmp/A /tmp/B
[10] message two
[11] message three
[12] message four
If we pre-trim the timestamps, the output is what we expect:
$ comm --nocheck-order -13 \
<(sed 's/^\[[ 0-9.]*\] //' /tmp/A) \
<(sed 's/^\[[ 0-9.]*\] //' /tmp/B)
message four
however, I'm not sure if that fix would easily apply. It looks like I
provided a disclaimer notice in check_result():
# Note: when comparing dmesg output, the kernel log timestamps
# help differentiate repeated testing runs. Remove them with a
# post-comparison sed filter.
so I wonder if comm will get confused with repeated selftest runs?
Using diff/comm was a trick that I surprised worked this long :) Maybe
it can still hold, but I'll have to run a few experiements.
--
Joe
Re: [PATCH net-next v10 08/10] net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in channel mode
On 18/12/2023 14:55, Roger Quadros wrote: > From: Grygorii Strashko > > This patch adds MQPRIO Qdisc offload in full 'channel' mode which allows > not only setting up pri:tc mapping, but also configuring TX shapers > (rate-limiting) on external port FIFOs. > > The MQPRIO Qdisc offload is expected to work with or without VLAN/priority > tagged packets. > > The CPSW external Port FIFO has 8 Priority queues. The rate-limit can be > set for each of these priority queues. Which Priority queue a packet is > assigned to depends on PN_REG_TX_PRI_MAP register which maps header > priority to switch priority. > > The header priority of a packet is assigned via the RX_PRI_MAP_REG which > maps packet priority to header priority. > > The packet priority is either the VLAN priority (for VLAN tagged packets) > or the thread/channel offset. > > For simplicity, we assign the same priority queue to all queues of a > Traffic Class so it can be rate-limited correctly. > > Configuration example: > ethtool -L eth1 tx 5 > ethtool --set-priv-flags eth1 p0-rx-ptype-rrobin off > > tc qdisc add dev eth1 parent root handle 100: mqprio num_tc 3 \ > map 0 0 1 2 0 0 0 0 0 0 0 0 0 0 0 0 \ > queues 1@0 1@1 1@2 hw 1 mode channel \ > shaper bw_rlimit min_rate 0 100mbit 200mbit max_rate 0 101mbit 202mbit > > tc qdisc replace dev eth2 handle 100: parent root mqprio num_tc 1 \ > map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 queues 1@0 hw 1 > > ip link add link eth1 name eth1.100 type vlan id 100 > ip link set eth1.100 type vlan egress 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7 > > In the above example two ports share the same TX CPPI queue 0 for low > priority traffic. 3 traffic classes are defined for eth1 and mapped to: > TC0 - low priority, TX CPPI queue 0 -> ext Port 1 fifo0, no rate limit > TC1 - prio 2, TX CPPI queue 1 -> ext Port 1 fifo1, CIR=100Mbit/s, EIR=1Mbit/s > TC2 - prio 3, TX CPPI queue 2 -> ext Port 1 fifo2, CIR=200Mbit/s, EIR=2Mbit/s > > Signed-off-by: Grygorii Strashko > Signed-off-by: Roger Quadros > Reviewed-by: Vladimir Oltean > --- > drivers/net/ethernet/ti/Kconfig | 3 +- > drivers/net/ethernet/ti/am65-cpsw-nuss.c | 3 + > drivers/net/ethernet/ti/am65-cpsw-qos.c | 255 ++- > drivers/net/ethernet/ti/am65-cpsw-qos.h | 20 ++ > 4 files changed, 277 insertions(+), 4 deletions(-) This breaks build due to undefined BYTES_PER_MBIT. I'll fix it up and send another revision. -- cheers, -roger
Re: [PATCH][next] selftests/net: Fix various spelling mistakes in TCP-AO tests
On 12/18/23 13:30, Colin Ian King wrote:
> There are a handful of spelling mistakes in test messages in the
> TCP-AIO selftests. Fix these.
>
> Signed-off-by: Colin Ian King
Thank you, Colin.
The patch looks good to me, my grammar and typos - not so much :-)
Reviewed-by: Dmitry Safonov
> ---
> tools/testing/selftests/net/tcp_ao/connect-deny.c | 2 +-
> tools/testing/selftests/net/tcp_ao/lib/proc.c | 4 ++--
> tools/testing/selftests/net/tcp_ao/setsockopt-closed.c | 2 +-
> tools/testing/selftests/net/tcp_ao/unsigned-md5.c | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/net/tcp_ao/connect-deny.c
> b/tools/testing/selftests/net/tcp_ao/connect-deny.c
> index 1ca78040d8b7..185a2f6e5ff3 100644
> --- a/tools/testing/selftests/net/tcp_ao/connect-deny.c
> +++ b/tools/testing/selftests/net/tcp_ao/connect-deny.c
> @@ -55,7 +55,7 @@ static void try_accept(const char *tst_name, unsigned int
> port, const char *pwd,
> err = test_wait_fd(lsk, timeout, 0);
> if (err == -ETIMEDOUT) {
> if (!fault(TIMEOUT))
> - test_fail("timeouted for accept()");
> + test_fail("timed out for accept()");
> } else if (err < 0) {
> test_error("test_wait_fd()");
> } else {
> diff --git a/tools/testing/selftests/net/tcp_ao/lib/proc.c
> b/tools/testing/selftests/net/tcp_ao/lib/proc.c
> index 2322f4d4676d..2fb6dd8adba6 100644
> --- a/tools/testing/selftests/net/tcp_ao/lib/proc.c
> +++ b/tools/testing/selftests/net/tcp_ao/lib/proc.c
> @@ -227,7 +227,7 @@ void netstat_print_diff(struct netstat *nsa, struct
> netstat *nsb)
> }
>
> if (nsb->counters_nr < nsa->counters_nr)
> - test_error("Unexpected: some counters dissapeared!");
> + test_error("Unexpected: some counters disappeared!");
>
> for (j = 0, i = 0; i < nsb->counters_nr; i++) {
> if (strcmp(nsb->counters[i].name,
> nsa->counters[j].name)) {
> @@ -244,7 +244,7 @@ void netstat_print_diff(struct netstat *nsa, struct
> netstat *nsb)
> j++;
> }
> if (j != nsa->counters_nr)
> - test_error("Unexpected: some counters dissapeared!");
> + test_error("Unexpected: some counters disappeared!");
>
> nsb = nsb->next;
> nsa = nsa->next;
> diff --git a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> index 7e4601b3f6a3..a329f42f40ce 100644
> --- a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> +++ b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> @@ -427,7 +427,7 @@ static void test_einval_del_key(void)
>
> sk = prepare_defs(TCP_AO_DEL_KEY, &del);
> del.set_current = 1;
> - setsockopt_checked(sk, TCP_AO_DEL_KEY, &del, ENOENT, "set non-exising
> current key");
> + setsockopt_checked(sk, TCP_AO_DEL_KEY, &del, ENOENT, "set non-existing
> current key");
>
> sk = prepare_defs(TCP_AO_DEL_KEY, &del);
> del.set_rnext = 1;
> diff --git a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> index 7cffde02d2be..14addfd46468 100644
> --- a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> +++ b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> @@ -72,7 +72,7 @@ static void try_accept(const char *tst_name, unsigned int
> port,
> err = test_wait_fd(lsk, timeout, 0);
> if (err == -ETIMEDOUT) {
> if (!fault(TIMEOUT))
> - test_fail("timeouted for accept()");
> + test_fail("timed out for accept()");
> } else if (err < 0) {
> test_error("test_wait_fd()");
> } else {
Thanks,
Dmitry
Re: [PATCH][next] selftests/net: Fix various spelling mistakes in TCP-AO tests
On 12/18/23 05:30, Colin Ian King wrote:
> There are a handful of spelling mistakes in test messages in the
> TCP-AIO selftests. Fix these.
>
> Signed-off-by: Colin Ian King
Reviewed-by: Randy Dunlap
Thanks.
> ---
> tools/testing/selftests/net/tcp_ao/connect-deny.c | 2 +-
> tools/testing/selftests/net/tcp_ao/lib/proc.c | 4 ++--
> tools/testing/selftests/net/tcp_ao/setsockopt-closed.c | 2 +-
> tools/testing/selftests/net/tcp_ao/unsigned-md5.c | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/net/tcp_ao/connect-deny.c
> b/tools/testing/selftests/net/tcp_ao/connect-deny.c
> index 1ca78040d8b7..185a2f6e5ff3 100644
> --- a/tools/testing/selftests/net/tcp_ao/connect-deny.c
> +++ b/tools/testing/selftests/net/tcp_ao/connect-deny.c
> @@ -55,7 +55,7 @@ static void try_accept(const char *tst_name, unsigned int
> port, const char *pwd,
> err = test_wait_fd(lsk, timeout, 0);
> if (err == -ETIMEDOUT) {
> if (!fault(TIMEOUT))
> - test_fail("timeouted for accept()");
> + test_fail("timed out for accept()");
> } else if (err < 0) {
> test_error("test_wait_fd()");
> } else {
> diff --git a/tools/testing/selftests/net/tcp_ao/lib/proc.c
> b/tools/testing/selftests/net/tcp_ao/lib/proc.c
> index 2322f4d4676d..2fb6dd8adba6 100644
> --- a/tools/testing/selftests/net/tcp_ao/lib/proc.c
> +++ b/tools/testing/selftests/net/tcp_ao/lib/proc.c
> @@ -227,7 +227,7 @@ void netstat_print_diff(struct netstat *nsa, struct
> netstat *nsb)
> }
>
> if (nsb->counters_nr < nsa->counters_nr)
> - test_error("Unexpected: some counters dissapeared!");
> + test_error("Unexpected: some counters disappeared!");
>
> for (j = 0, i = 0; i < nsb->counters_nr; i++) {
> if (strcmp(nsb->counters[i].name,
> nsa->counters[j].name)) {
> @@ -244,7 +244,7 @@ void netstat_print_diff(struct netstat *nsa, struct
> netstat *nsb)
> j++;
> }
> if (j != nsa->counters_nr)
> - test_error("Unexpected: some counters dissapeared!");
> + test_error("Unexpected: some counters disappeared!");
>
> nsb = nsb->next;
> nsa = nsa->next;
> diff --git a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> index 7e4601b3f6a3..a329f42f40ce 100644
> --- a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> +++ b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c
> @@ -427,7 +427,7 @@ static void test_einval_del_key(void)
>
> sk = prepare_defs(TCP_AO_DEL_KEY, &del);
> del.set_current = 1;
> - setsockopt_checked(sk, TCP_AO_DEL_KEY, &del, ENOENT, "set non-exising
> current key");
> + setsockopt_checked(sk, TCP_AO_DEL_KEY, &del, ENOENT, "set non-existing
> current key");
>
> sk = prepare_defs(TCP_AO_DEL_KEY, &del);
> del.set_rnext = 1;
> diff --git a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> index 7cffde02d2be..14addfd46468 100644
> --- a/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> +++ b/tools/testing/selftests/net/tcp_ao/unsigned-md5.c
> @@ -72,7 +72,7 @@ static void try_accept(const char *tst_name, unsigned int
> port,
> err = test_wait_fd(lsk, timeout, 0);
> if (err == -ETIMEDOUT) {
> if (!fault(TIMEOUT))
> - test_fail("timeouted for accept()");
> + test_fail("timed out for accept()");
> } else if (err < 0) {
> test_error("test_wait_fd()");
> } else {
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
Re: Should I add BPF kfuncs for userspace apps? And how?
Hi Akihiko, On Tue, Dec 12, 2023 at 12:05 AM Akihiko Odaki wrote: > [...] > --- > > I'm working on a new feature that aids virtio-net implementations using > tuntap virtual network device. You can see [1] for details, but > basically it's to extend BPF_PROG_TYPE_SOCKET_FILTER to report four more > bytes. AFAICT, [1] adds a new program type, which is really hard to ship. However, you mentioned it is basically "extend BPF_PROG_TYPE_SOCKET_FILTER to report four more bytes", which confuses me. Can we achieve the same goal by extending BPF_PROG_TYPE_SOCKET_FILTER (without adding a new program type)? Does this require extending __sk_buff, which is also not an option any more? Thanks, Song
Re: [PATCH v3 3/3] selftests: livepatch: Test livepatching a heavily called syscall
On 12/15/23 13:36, Joe Lawrence wrote: On 12/11/23 16:53, Shuah Khan wrote: On 12/6/23 07:39, Miroslav Benes wrote: Hi, On Tue, 5 Dec 2023, Shuah Khan wrote: On 12/5/23 05:52, mpdeso...@suse.com wrote: On Fri, 2023-12-01 at 16:38 +, Shuah Khan wrote: 0003-selftests-livepatch-Test-livepatching-a-heavily-call.patch has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. I couldn't find any mention about "missing module name". Is your script showing more warnings than these ones? Can you please share your output? I'll fix MAINTAINERS file but I'll wait until I understand what's missing in your checkpatch script to resend the patchset. Looks like it is coming a script - still my question stands on whether or not you would need a module name for this module? I admit I am also clueless here. The module name is given in Makefile. In this case in test_modules/Makefile. I do not know of anything else. There is no MODULE_NAME macro. Could you elaborate, please? I see that now. Hi Shuah, In the other replies to this thread, Marcos noted that he would add some text to the commit / documentation on running and building the selftests directly in the kernel tree (that would get my Ack) ... is there anything else to be updated for a hopefully final v4 (for your Ack)? I am waiting for v4 with your comments are addressed. I can take this through kselftest tree. thanks, -- Shuah
Re: [PATCH v4] tracing/selftests: Add test to test the trace_marker
On 12/13/23 15:59, Masami Hiramatsu (Google) wrote: On Wed, 13 Dec 2023 11:15:27 -0500 Steven Rostedt wrote: From: "Steven Rostedt (Google)" Add a test that writes longs strings, some over the size of the sub buffer and make sure that the entire content is there. This looks good to me. and tested. Acked-by: Masami Hiramatsu (Google) Tested-by: Masami Hiramatsu (Google) Assuming this is going through tracing: Acked-by: Shuah Khan If not just let me know, I will take this through kselftest tree. thanks, -- Shuah
Re: [PATCH v4] tracing/selftests: Add test to test the trace_marker
On Mon, 18 Dec 2023 13:49:22 -0700 Shuah Khan wrote: > On 12/13/23 15:59, Masami Hiramatsu (Google) wrote: > > On Wed, 13 Dec 2023 11:15:27 -0500 > > Steven Rostedt wrote: > > > >> From: "Steven Rostedt (Google)" > >> > >> Add a test that writes longs strings, some over the size of the sub buffer > >> and make sure that the entire content is there. > > > > This looks good to me. and tested. > > > > Acked-by: Masami Hiramatsu (Google) > > Tested-by: Masami Hiramatsu (Google) > > > > Assuming this is going through tracing: Yeah, I'll pull it through mine, as it's being used to make sure the code I'm working on still works. > > Acked-by: Shuah Khan Thanks, I'll add that. -- Steve
Re: selftests/livepatch fails on s390
On Mon, Dec 18, 2023 at 11:44:23AM -0500, Joe Lawrence wrote:
>
> [ ... snip ... ]
>
> If we pre-trim the timestamps, the output is what we expect:
>
> $ comm --nocheck-order -13 \
> <(sed 's/^\[[ 0-9.]*\] //' /tmp/A) \
> <(sed 's/^\[[ 0-9.]*\] //' /tmp/B)
> message four
>
> however, I'm not sure if that fix would easily apply. It looks like I
> provided a disclaimer notice in check_result():
>
> # Note: when comparing dmesg output, the kernel log timestamps
> # help differentiate repeated testing runs. Remove them with a
> # post-comparison sed filter.
>
> so I wonder if comm will get confused with repeated selftest runs?
> Using diff/comm was a trick that I surprised worked this long :) Maybe
> it can still hold, but I'll have to run a few experiements.
Hi Alexander,
I tested this idea and yeah, it breaks. Google brought me back to
Petr's report from when way back when I tried to omit the timestamps:
https://lore.kernel.org/all/20200617092352.GT31238@alley/
that was with diff and not comm, but I could trip up the latter command
with ~22 iterations of the selftests.
So I took a crack at refactoring: instead of saving intermediate
dmesg.saved files, on starting a test, the script logs and remembers a
LAST_DMESG entry. When it later checks the test result, it dumps dmesg
starting from after LAST_DMESG entry.
This is *very* lightly tested, but I thought maybe you could give it a
spin. Hopefully it's less brittle than diff/comm strategy.
-- Joe
-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--
diff --git a/tools/testing/selftests/livepatch/functions.sh
b/tools/testing/selftests/livepatch/functions.sh
index c8416c54b463..b012723e631a 100644
--- a/tools/testing/selftests/livepatch/functions.sh
+++ b/tools/testing/selftests/livepatch/functions.sh
@@ -42,17 +42,6 @@ function die() {
exit 1
}
-# save existing dmesg so we can detect new content
-function save_dmesg() {
- SAVED_DMESG=$(mktemp --tmpdir -t klp-dmesg-XX)
- dmesg > "$SAVED_DMESG"
-}
-
-# cleanup temporary dmesg file from save_dmesg()
-function cleanup_dmesg_file() {
- rm -f "$SAVED_DMESG"
-}
-
function push_config() {
DYNAMIC_DEBUG=$(grep '^kernel/livepatch'
/sys/kernel/debug/dynamic_debug/control | \
awk -F'[: ]' '{print "file " $1 " line " $2 " " $4}')
@@ -99,7 +88,6 @@ function set_ftrace_enabled() {
function cleanup() {
pop_config
- cleanup_dmesg_file
}
# setup_config - save the current config and set a script exit trap that
@@ -280,7 +268,13 @@ function set_pre_patch_ret {
function start_test {
local test="$1"
- save_dmesg
+ # Dump something unique into the dmesg log, then stash the entry
+ # in LAST_DMESG. The check_result() function will use it to
+ # find new kernel messages since the test started.
+ local timestamp="$(date --rfc-3339=ns)"
+ log "livepatch kselftest timestamp: $timestamp"
+ LAST_DMESG=$(dmesg | grep "livepatch kselftest timestamp: $timestamp")
+
echo -n "TEST: $test ... "
log "= TEST: $test ="
}
@@ -291,11 +285,11 @@ function check_result {
local expect="$*"
local result
- # Note: when comparing dmesg output, the kernel log timestamps
- # help differentiate repeated testing runs. Remove them with a
- # post-comparison sed filter.
-
- result=$(dmesg | comm --nocheck-order -13 "$SAVED_DMESG" - | \
+ # Test results include any new dmesg entry since LAST_DMESG, then:
+ # - include lines matching keywords
+ # - exclude lines matching keywords
+ # - filter out dmesg timestamp prefixes
+ result=$(dmesg | awk -v last_dmesg="$LAST_DMESG" 'p; $0 == last_dmesg {
p=1 }' | \
grep -e 'livepatch:' -e 'test_klp' | \
grep -v '\(tainting\|taints\) kernel' | \
sed 's/^\[[ 0-9.]*\] //')
@@ -306,8 +300,6 @@ function check_result {
echo -e "not ok\n\n$(diff -upr --label expected --label result
<(echo "$expect") <(echo "$result"))\n"
die "livepatch kselftest(s) failed"
fi
-
- cleanup_dmesg_file
}
# check_sysfs_rights(modname, rel_path, expected_rights) - check sysfs
[PATCH] kselftest/arm64: Don't probe the current VL for unsupported vector types
The vec-syscfg selftest verifies that setting the VL of the currently
tested vector type does not disrupt the VL of the other vector type. To do
this it records the current vector length for each type but neglects to
guard this with a check for that vector type actually being supported. Add
one, using a helper function which we also update all the other instances
of this pattern.
Signed-off-by: Mark Brown
---
tools/testing/selftests/arm64/fp/vec-syscfg.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/arm64/fp/vec-syscfg.c
b/tools/testing/selftests/arm64/fp/vec-syscfg.c
index 5f648b97a06f..ea9c7d47790f 100644
--- a/tools/testing/selftests/arm64/fp/vec-syscfg.c
+++ b/tools/testing/selftests/arm64/fp/vec-syscfg.c
@@ -66,6 +66,11 @@ static struct vec_data vec_data[] = {
},
};
+static bool vec_type_supported(struct vec_data *data)
+{
+ return getauxval(data->hwcap_type) & data->hwcap;
+}
+
static int stdio_read_integer(FILE *f, const char *what, int *val)
{
int n = 0;
@@ -564,8 +569,11 @@ static void prctl_set_all_vqs(struct vec_data *data)
return;
}
- for (i = 0; i < ARRAY_SIZE(vec_data); i++)
+ for (i = 0; i < ARRAY_SIZE(vec_data); i++) {
+ if (!vec_type_supported(&vec_data[i]))
+ continue;
orig_vls[i] = vec_data[i].rdvl();
+ }
for (vq = SVE_VQ_MIN; vq <= SVE_VQ_MAX; vq++) {
vl = sve_vl_from_vq(vq);
@@ -594,7 +602,7 @@ static void prctl_set_all_vqs(struct vec_data *data)
if (&vec_data[i] == data)
continue;
- if (!(getauxval(vec_data[i].hwcap_type) &
vec_data[i].hwcap))
+ if (!vec_type_supported(&vec_data[i]))
continue;
if (vec_data[i].rdvl() != orig_vls[i]) {
@@ -765,7 +773,7 @@ int main(void)
struct vec_data *data = &vec_data[i];
unsigned long supported;
- supported = getauxval(data->hwcap_type) & data->hwcap;
+ supported = vec_type_supported(data);
if (!supported)
all_supported = false;
---
base-commit: 2cc14f52aeb78ce3f29677c2de1f06c0e91471ab
change-id: 20231215-kselftest-arm64-vec-syscfg-rdvl-7944e19ac64f
Best regards,
--
Mark Brown
Re: [PATCH v1] selftests/mm: Log run_vmtests.sh results in TAP format
On 12/18/23 03:32, Ryan Roberts wrote: ... I should also point out that some of the subtests already attempt a TAP output. So now we end up with TAP-within-TAP output for those programs. It's actually TAP-in-TAP-in-TAP if you're running from run_kselftest.sh :) For example: # --- # running ./madv_populate # --- # TAP version 13 # 1..21 # # [RUN] test_prot_read # ok 1 MADV_POPULATE_READ with PROT_READ # ok 2 MADV_POPULATE_WRITE with PROT_READ # # [RUN] test_prot_write # ok 3 MADV_POPULATE_READ with PROT_WRITE ...etc... Note the double level of leading '#' characters. Again, this is still readable enough for humans. But it should probably be removed in subsequent patches to the subtests. I personally don't agree with this. It would be difficult to flatten to a single TAP instance because the top level doesn't have a clue how many test cases the That's not quite what I had in mind... child is running. Trying to do this will make things more fragile and less modular. LAVA can certainly deal with nested test cases and correctly parses everything to test case names that contain the test name at each level of nesting. The thing I was trying to solve with this patch was that previously the top level (run_kselftest.sh) and the bottom level (individual mm test binaries) were using TAP, but the middle level (run_vmtests.sh) wasn't, and this was confusing the LAVA parser. I was thinking more along these lines: a) For the individual programs (binaries), there is actually neither need nor desire to create TAP output at that level, because frameworks like LAVA only care about running a lot of tests and parsing the output. b) Therefore, just stop specifying TAP output at the leaf level, and let run_vmtests.sh and run_kselftest.sh do it. Looking at madv_populate.c, I see that it scatters calls to ksft_*() around. And I was thinking that this is all just redundant, isn't it? thanks, -- John Hubbard NVIDIA
Re: [PATCH v1] selftests/mm: Log run_vmtests.sh results in TAP format
On 12/18/23 16:51, John Hubbard wrote: On 12/18/23 03:32, Ryan Roberts wrote: ... I should also point out that some of the subtests already attempt a TAP output. So now we end up with TAP-within-TAP output for those programs. It's actually TAP-in-TAP-in-TAP if you're running from run_kselftest.sh :) For example: # --- # running ./madv_populate # --- # TAP version 13 # 1..21 # # [RUN] test_prot_read # ok 1 MADV_POPULATE_READ with PROT_READ # ok 2 MADV_POPULATE_WRITE with PROT_READ # # [RUN] test_prot_write # ok 3 MADV_POPULATE_READ with PROT_WRITE ...etc... Note the double level of leading '#' characters. Again, this is still readable enough for humans. But it should probably be removed in subsequent patches to the subtests. I personally don't agree with this. It would be difficult to flatten to a single TAP instance because the top level doesn't have a clue how many test cases the That's not quite what I had in mind... child is running. Trying to do this will make things more fragile and less modular. LAVA can certainly deal with nested test cases and correctly parses everything to test case names that contain the test name at each level of nesting. The thing I was trying to solve with this patch was that previously the top level (run_kselftest.sh) and the bottom level (individual mm test binaries) were using TAP, but the middle level (run_vmtests.sh) wasn't, and this was confusing the LAVA parser. I was thinking more along these lines: a) For the individual programs (binaries), there is actually neither need nor desire to create TAP output at that level, because frameworks like LAVA only care about running a lot of tests and parsing the output. b) Therefore, just stop specifying TAP output at the leaf level, and let run_vmtests.sh and run_kselftest.sh do it. Looking at madv_populate.c, I see that it scatters calls to ksft_*() around. And I was thinking that this is all just redundant, isn't it? Although I suppose that the counter argument is that the subtests in madv_populate.c really *do* want to be specifically printed in TAP format. arggh, I guess this is just not worth fooling around with after all. thanks, -- John Hubbard NVIDIA
[PATCH net-next] selftest/tcp-ao: Rectify out-of-tree build
Trivial fix for out-of-tree build that I wasn't testing previously:
1. Create a directory for library object files, fixes:
> gcc lib/kconfig.c -Wall -O2 -g -D_GNU_SOURCE -fno-strict-aliasing -I
> ../../../../../usr/include/ -iquote /tmp/kselftest/kselftest/net/tcp_ao/lib
> -I ../../../../include/ -o /tmp/kselftest/kselftest/net/tcp_ao/lib/kconfig.o
> -c
> Assembler messages:
> Fatal error: can't create /tmp/kselftest/kselftest/net/tcp_ao/lib/kconfig.o:
> No such file or directory
> make[1]: *** [Makefile:46: /tmp/kselftest/kselftest/net/tcp_ao/lib/kconfig.o]
> Error 1
2. Include $(KHDR_INCLUDES) that's exported by selftests/Makefile, fixes:
> In file included from lib/kconfig.c:6:
> lib/aolib.h:320:45: warning: ‘struct tcp_ao_add’ declared inside parameter
> list will not be visible outside of this definition or declaration
> 320 | extern int test_prepare_key_sockaddr(struct tcp_ao_add *ao, const
> char *alg,
> | ^~
...
3. While at here, clean-up $(KSFT_KHDR_INSTALL): it's not needed anymore
since commit f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
4. Also, while at here, drop .DEFAULT_GOAL definition: that has a
self-explaining comment, that was valid when I made these selftests
compile on local v4.19 kernel, but not needed since
commit 8ce72dc32578 ("selftests: fix headers_install circular dependency")
Fixes: cfbab37b3da0 ("selftests/net: Add TCP-AO library")
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202312190645.q76mmhyq-...@intel.com/
Signed-off-by: Dmitry Safonov
---
tools/testing/selftests/net/tcp_ao/Makefile | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/net/tcp_ao/Makefile
b/tools/testing/selftests/net/tcp_ao/Makefile
index 6343cfcf919b..8e60bae67aa9 100644
--- a/tools/testing/selftests/net/tcp_ao/Makefile
+++ b/tools/testing/selftests/net/tcp_ao/Makefile
@@ -17,22 +17,18 @@ TEST_IPV6_PROGS := $(TEST_BOTH_AF:%=%_ipv6)
TEST_GEN_PROGS := $(TEST_IPV4_PROGS) $(TEST_IPV6_PROGS)
top_srcdir := ../../../../..
-KSFT_KHDR_INSTALL := 1
include ../../lib.mk
HOSTAR ?= ar
-# Drop it on port to linux/master with commit 8ce72dc32578
-.DEFAULT_GOAL := all
-
LIBDIR := $(OUTPUT)/lib
LIB:= $(LIBDIR)/libaotst.a
LDLIBS += $(LIB) -pthread
LIBDEPS:= lib/aolib.h Makefile
CFLAGS := -Wall -O2 -g -D_GNU_SOURCE -fno-strict-aliasing
-CFLAGS += -I ../../../../../usr/include/ -iquote $(LIBDIR)
-CFLAGS += -I ../../../../include/
+CFLAGS += $(KHDR_INCLUDES)
+CFLAGS += -iquote ./lib/ -I ../../../../include/
# Library
LIBSRC := kconfig.c netlink.c proc.c repair.c setup.c sock.c utils.c
@@ -43,6 +39,7 @@ $(LIB): $(LIBOBJ)
$(HOSTAR) rcs $@ $^
$(LIBDIR)/%.o: ./lib/%.c $(LIBDEPS)
+ mkdir -p $(LIBDIR)
$(CC) $< $(CFLAGS) $(CPPFLAGS) -o $@ -c
$(TEST_GEN_PROGS): $(LIB)
---
base-commit: ceb2fe0d438644e1de06b9a6468a1fb8e2199c70
change-id: 20231219-b4-tcp-ao-selftests-out-of-tree-452f787f2d58
Best regards,
--
Dmitry Safonov
Re: [PATCH v4 08/11] KVM: riscv: selftests: Add guest helper to get vcpu id
On Wed, Dec 13, 2023 at 10:08 PM Andrew Jones wrote:
>
> On Tue, Dec 12, 2023 at 05:31:17PM +0800, Haibo Xu wrote:
> > Add guest_get_vcpuid() helper to simplify accessing to per-cpu
> > private data. The sscratch CSR was used to store the vcpu id.
> >
> > Signed-off-by: Haibo Xu
> > Reviewed-by: Andrew Jones
> > ---
> > tools/testing/selftests/kvm/include/aarch64/processor.h | 4
> > tools/testing/selftests/kvm/include/kvm_util_base.h | 9 +
> > tools/testing/selftests/kvm/lib/riscv/processor.c | 8
> > 3 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h
> > b/tools/testing/selftests/kvm/include/aarch64/processor.h
> > index c42d683102c7..16ae0ac01879 100644
> > --- a/tools/testing/selftests/kvm/include/aarch64/processor.h
> > +++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
> > @@ -226,8 +226,4 @@ void smccc_smc(uint32_t function_id, uint64_t arg0,
> > uint64_t arg1,
> > uint64_t arg2, uint64_t arg3, uint64_t arg4, uint64_t arg5,
> > uint64_t arg6, struct arm_smccc_res *res);
> >
> > -
> > -
> > -uint32_t guest_get_vcpuid(void);
> > -
> > #endif /* SELFTEST_KVM_PROCESSOR_H */
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h
> > b/tools/testing/selftests/kvm/include/kvm_util_base.h
> > index a18db6a7b3cf..666438113d22 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> > @@ -932,4 +932,13 @@ void kvm_selftest_arch_init(void);
> >
> > void kvm_arch_vm_post_create(struct kvm_vm *vm);
> >
> > +void vm_init_vector_tables(struct kvm_vm *vm);
> > +void vcpu_init_vector_tables(struct kvm_vcpu *vcpu);
> > +
> > +struct ex_regs;
> > +typedef void(*exception_handler_fn)(struct ex_regs *);
> > +void vm_install_exception_handler(struct kvm_vm *vm, int vector,
> > exception_handler_fn handler);
>
> I think something happend on rebase since the above should be in a
> different patch (and were for v3). I suggest checking your previous
> and current version branches with git-range-diff after rebasing in
> order to catch stuff like this.
>
> Thanks,
> drew
>
Thanks for pointing it out. Will fix it in v5.
> > +
> > +uint32_t guest_get_vcpuid(void);
> > +
> > #endif /* SELFTEST_KVM_UTIL_BASE_H */
> > diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c
> > b/tools/testing/selftests/kvm/lib/riscv/processor.c
> > index efd9ac4b0198..39a1e9902dec 100644
> > --- a/tools/testing/selftests/kvm/lib/riscv/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/riscv/processor.c
> > @@ -316,6 +316,9 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm,
> > uint32_t vcpu_id,
> > vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.sp), stack_vaddr + stack_size);
> > vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), (unsigned
> > long)guest_code);
> >
> > + /* Setup sscratch for guest_get_vcpuid() */
> > + vcpu_set_reg(vcpu, RISCV_CSR_REG(sscratch), vcpu_id);
> > +
> > /* Setup default exception vector of guest */
> > vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned
> > long)guest_unexp_trap);
> >
> > @@ -436,3 +439,8 @@ void vm_install_interrupt_handler(struct kvm_vm *vm,
> > exception_handler_fn handle
> >
> > handlers->exception_handlers[1][0] = handler;
> > }
> > +
> > +uint32_t guest_get_vcpuid(void)
> > +{
> > + return csr_read(CSR_SSCRATCH);
> > +}
> > --
> > 2.34.1
> >
[PATCH] selftests/net: remove unneeded semicolon
No functional modification involved. ./tools/testing/selftests/net/tcp_ao/setsockopt-closed.c:121:2-3: Unneeded semicolon. Reported-by: Abaci Robot Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7771 Signed-off-by: Jiapeng Chong --- tools/testing/selftests/net/tcp_ao/setsockopt-closed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c index 7e4601b3f6a3..d5d0484b5d44 100644 --- a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c +++ b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c @@ -118,7 +118,7 @@ static void setsockopt_checked(int sk, int optname, void *optval, break; default: break; - }; + } __setsockopt_checked(sk, optname, false, optval, &len, err, cmd, tst); } -- 2.20.1.7.g153144c
