2023-09-22 18:49:54

by Lukasz Majewski

[permalink] [raw]
Subject: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

This patch series provides support for HSR HW offloading in KSZ9477
switch IC.

To test this feature:
ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
ip link set dev lan1 up
ip link set dev lan2 up
ip a add 192.168.0.1/24 dev hsr0
ip link set dev hsr0 up

To remove HSR network device:
ip link del hsr0

To test if one can adjust MAC address:
ip link set lan2 address 00:01:02:AA:BB:CC

It is also possible to create another HSR interface, but it will
only support HSR is software - e.g.
ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision 45 version 1

Test HW:
Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".

Performance SW used:
nuttcp -S --nofork
nuttcp -vv -T 60 -r 192.168.0.2
nuttcp -vv -T 60 -t 192.168.0.2

Code: v6.6.0-rc2+ Linux net-next repository
SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172

Tested HSR v0 and v1
Results:
With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
With no offloading RX: 63 Mbps TX: 63 Mbps


Lukasz Majewski (2):
net: dsa: tag_ksz: Extend ksz9477_xmit() for HSR frame duplication
net: dsa: microchip: Enable HSR offloading for KSZ9477

Vladimir Oltean (3):
net: dsa: propagate extack to ds->ops->port_hsr_join()
net: dsa: notify drivers of MAC address changes on user ports
net: dsa: microchip: move REG_SW_MAC_ADDR to dev->info->regs[]

drivers/net/dsa/microchip/ksz8795_reg.h | 7 --
drivers/net/dsa/microchip/ksz9477.c | 77 ++++++++++++
drivers/net/dsa/microchip/ksz9477.h | 2 +
drivers/net/dsa/microchip/ksz9477_reg.h | 7 --
drivers/net/dsa/microchip/ksz_common.c | 149 ++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_common.h | 10 ++
drivers/net/dsa/xrs700x/xrs700x.c | 18 ++-
include/net/dsa.h | 13 ++-
net/dsa/port.c | 5 +-
net/dsa/port.h | 3 +-
net/dsa/slave.c | 9 +-
net/dsa/tag_ksz.c | 8 ++
12 files changed, 283 insertions(+), 25 deletions(-)

--
2.20.1


2023-09-22 21:49:58

by Lukasz Majewski

[permalink] [raw]
Subject: [PATCH v6 net-next 2/5] net: dsa: notify drivers of MAC address changes on user ports

From: Vladimir Oltean <[email protected]>

In some cases, drivers may need to veto the changing of a MAC address on
a user port. Such is the case with KSZ9477 when it offloads a HSR device,
because it programs the MAC address of multiple ports to a shared
hardware register. Those ports need to have equal MAC addresses for the
lifetime of the HSR offload.

Signed-off-by: Vladimir Oltean <[email protected]>
Signed-off-by: Lukasz Majewski <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
Changes for v5:
- New patch
Changes for v6:
- None
---
include/net/dsa.h | 10 ++++++++++
net/dsa/slave.c | 7 +++++++
2 files changed, 17 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 426724808e76..d98439ea6146 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -969,6 +969,16 @@ struct dsa_switch_ops {
struct phy_device *phy);
void (*port_disable)(struct dsa_switch *ds, int port);

+
+ /*
+ * Notification for MAC address changes on user ports. Drivers can
+ * currently only veto operations. They should not use the method to
+ * program the hardware, since the operation is not rolled back in case
+ * of other errors.
+ */
+ int (*port_set_mac_address)(struct dsa_switch *ds, int port,
+ const unsigned char *addr);
+
/*
* Compatibility between device trees defining multiple CPU ports and
* drivers which are not OK to use by default the numerically smallest
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2b3d89b77121..4c3e502d7e16 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -457,6 +457,13 @@ static int dsa_slave_set_mac_address(struct net_device *dev, void *a)
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;

+ if (ds->ops->port_set_mac_address) {
+ err = ds->ops->port_set_mac_address(ds, dp->index,
+ addr->sa_data);
+ if (err)
+ return err;
+ }
+
/* If the port is down, the address isn't synced yet to hardware or
* to the DSA master, so there is nothing to change.
*/
--
2.20.1

2023-09-23 02:21:11

by Lukasz Majewski

[permalink] [raw]
Subject: [PATCH v6 net-next 4/5] net: dsa: microchip: move REG_SW_MAC_ADDR to dev->info->regs[]

From: Vladimir Oltean <[email protected]>

Defining macros which have the same name but different values is bad
practice, because it makes it hard to avoid code duplication. The same
code does different things, depending on the file it's placed in.
Case in point, we want to access REG_SW_MAC_ADDR from ksz_common.c, but
currently we can't, because we don't know which kszXXXX_reg.h to include
from the common code.

Remove the REG_SW_MAC_ADDR_{0..5} macros from ksz8795_reg.h and
ksz9477_reg.h, and re-add this register offset to the dev->info->regs[]
array.

Signed-off-by: Vladimir Oltean <[email protected]>
Signed-off-by: Lukasz Majewski <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
Changes for v5:
- New patch
Changes for v6:
- None
---
drivers/net/dsa/microchip/ksz8795_reg.h | 7 -------
drivers/net/dsa/microchip/ksz9477_reg.h | 7 -------
drivers/net/dsa/microchip/ksz_common.c | 2 ++
drivers/net/dsa/microchip/ksz_common.h | 1 +
4 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index d33db4f86c64..3c9dae53e4d8 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -323,13 +323,6 @@
((addr) + REG_PORT_1_CTRL_0 + (port) * \
(REG_PORT_2_CTRL_0 - REG_PORT_1_CTRL_0))

-#define REG_SW_MAC_ADDR_0 0x68
-#define REG_SW_MAC_ADDR_1 0x69
-#define REG_SW_MAC_ADDR_2 0x6A
-#define REG_SW_MAC_ADDR_3 0x6B
-#define REG_SW_MAC_ADDR_4 0x6C
-#define REG_SW_MAC_ADDR_5 0x6D
-
#define TABLE_EXT_SELECT_S 5
#define TABLE_EEE_V 1
#define TABLE_ACL_V 2
diff --git a/drivers/net/dsa/microchip/ksz9477_reg.h b/drivers/net/dsa/microchip/ksz9477_reg.h
index 504e085aab52..f3a205ee483f 100644
--- a/drivers/net/dsa/microchip/ksz9477_reg.h
+++ b/drivers/net/dsa/microchip/ksz9477_reg.h
@@ -153,13 +153,6 @@
#define SW_DOUBLE_TAG BIT(7)
#define SW_RESET BIT(1)

-#define REG_SW_MAC_ADDR_0 0x0302
-#define REG_SW_MAC_ADDR_1 0x0303
-#define REG_SW_MAC_ADDR_2 0x0304
-#define REG_SW_MAC_ADDR_3 0x0305
-#define REG_SW_MAC_ADDR_4 0x0306
-#define REG_SW_MAC_ADDR_5 0x0307
-
#define REG_SW_MTU__2 0x0308
#define REG_SW_MTU_MASK GENMASK(13, 0)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 173ad8f04671..6c31d51410e3 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -364,6 +364,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
};

static const u16 ksz8795_regs[] = {
+ [REG_SW_MAC_ADDR] = 0x68,
[REG_IND_CTRL_0] = 0x6E,
[REG_IND_DATA_8] = 0x70,
[REG_IND_DATA_CHECK] = 0x72,
@@ -492,6 +493,7 @@ static u8 ksz8863_shifts[] = {
};

static const u16 ksz9477_regs[] = {
+ [REG_SW_MAC_ADDR] = 0x0302,
[P_STP_CTRL] = 0x0B04,
[S_START_CTRL] = 0x0300,
[S_BROADCAST_CTRL] = 0x0332,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index d180c8a34e27..07c7723dbc37 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -212,6 +212,7 @@ enum ksz_chip_id {
};

enum ksz_regs {
+ REG_SW_MAC_ADDR,
REG_IND_CTRL_0,
REG_IND_DATA_8,
REG_IND_DATA_CHECK,
--
2.20.1

2023-09-27 05:44:55

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

On Fri, Sep 22, 2023 at 03:31:03PM +0200, Lukasz Majewski wrote:
> This patch series provides support for HSR HW offloading in KSZ9477
> switch IC.
>
> To test this feature:
> ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
> ip link set dev lan1 up
> ip link set dev lan2 up
> ip a add 192.168.0.1/24 dev hsr0
> ip link set dev hsr0 up
>
> To remove HSR network device:
> ip link del hsr0
>
> To test if one can adjust MAC address:
> ip link set lan2 address 00:01:02:AA:BB:CC
>
> It is also possible to create another HSR interface, but it will
> only support HSR is software - e.g.
> ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision 45 version 1
>
> Test HW:
> Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".
>
> Performance SW used:
> nuttcp -S --nofork
> nuttcp -vv -T 60 -r 192.168.0.2
> nuttcp -vv -T 60 -t 192.168.0.2
>
> Code: v6.6.0-rc2+ Linux net-next repository
> SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
>
> Tested HSR v0 and v1
> Results:
> With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
> With no offloading RX: 63 Mbps TX: 63 Mbps

Reviewed-by: Vladimir Oltean <[email protected]>

Thanks!

2023-09-28 11:43:07

by Lukasz Majewski

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

Hi Vladimir,

> On Fri, Sep 22, 2023 at 03:31:03PM +0200, Lukasz Majewski wrote:
> > This patch series provides support for HSR HW offloading in KSZ9477
> > switch IC.
> >
> > To test this feature:
> > ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision
> > 45 version 1 ip link set dev lan1 up
> > ip link set dev lan2 up
> > ip a add 192.168.0.1/24 dev hsr0
> > ip link set dev hsr0 up
> >
> > To remove HSR network device:
> > ip link del hsr0
> >
> > To test if one can adjust MAC address:
> > ip link set lan2 address 00:01:02:AA:BB:CC
> >
> > It is also possible to create another HSR interface, but it will
> > only support HSR is software - e.g.
> > ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision
> > 45 version 1
> >
> > Test HW:
> > Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".
> >
> > Performance SW used:
> > nuttcp -S --nofork
> > nuttcp -vv -T 60 -r 192.168.0.2
> > nuttcp -vv -T 60 -t 192.168.0.2
> >
> > Code: v6.6.0-rc2+ Linux net-next repository
> > SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
> >
> > Tested HSR v0 and v1
> > Results:
> > With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
> > With no offloading RX: 63 Mbps TX: 63 Mbps
>
> Reviewed-by: Vladimir Oltean <[email protected]>
>
> Thanks!

I hope, that it will find its way to net-next soon :-).

Thanks for your help and patience.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2023-10-03 07:58:55

by Lukasz Majewski

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

Hi Vladimir, Andrew, Woojung,

> Hi Vladimir,
>
> > On Fri, Sep 22, 2023 at 03:31:03PM +0200, Lukasz Majewski wrote:
> > > This patch series provides support for HSR HW offloading in
> > > KSZ9477 switch IC.
> > >
> > > To test this feature:
> > > ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision
> > > 45 version 1 ip link set dev lan1 up
> > > ip link set dev lan2 up
> > > ip a add 192.168.0.1/24 dev hsr0
> > > ip link set dev hsr0 up
> > >
> > > To remove HSR network device:
> > > ip link del hsr0
> > >
> > > To test if one can adjust MAC address:
> > > ip link set lan2 address 00:01:02:AA:BB:CC
> > >
> > > It is also possible to create another HSR interface, but it will
> > > only support HSR is software - e.g.
> > > ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision
> > > 45 version 1
> > >
> > > Test HW:
> > > Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".
> > >
> > > Performance SW used:
> > > nuttcp -S --nofork
> > > nuttcp -vv -T 60 -r 192.168.0.2
> > > nuttcp -vv -T 60 -t 192.168.0.2
> > >
> > > Code: v6.6.0-rc2+ Linux net-next repository
> > > SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
> > >
> > > Tested HSR v0 and v1
> > > Results:
> > > With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
> > > With no offloading RX: 63 Mbps TX: 63
> > > Mbps
> >
> > Reviewed-by: Vladimir Oltean <[email protected]>
> >
> > Thanks!
>
> I hope, that it will find its way to net-next soon :-).
>

I'm a bit puzzled with this patch series - will it be pulled directly
to net-next [1] or is there any other (KSZ maintainer's?) tree to which
it will be first pulled and then PR will be send to net-next?

Thanks in advance for the clarification.

> Thanks for your help and patience.
>

Links:

[1] -
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/log/

>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH, Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> [email protected]

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2023-10-03 10:44:30

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

On Tue, Oct 03, 2023 at 09:58:32AM +0200, Lukasz Majewski wrote:
> I'm a bit puzzled with this patch series - will it be pulled directly
> to net-next [1] or is there any other (KSZ maintainer's?) tree to which
> it will be first pulled and then PR will be send to net-next?
>
> Thanks in advance for the clarification.
>
> Links:
>
> [1] -
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/log/

No, there's no other tree than net-next. I see your patch was marked as
"Changes requested", let me see if I can transition it back to "Under review"
so that it gains the netdev maintainers' attention again:

https://patchwork.kernel.org/project/netdevbpf/cover/[email protected]/

pw-bot: under-review

2023-10-03 12:51:23

by Lukasz Majewski

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

Hi Vladimir,

> On Tue, Oct 03, 2023 at 09:58:32AM +0200, Lukasz Majewski wrote:
> > I'm a bit puzzled with this patch series - will it be pulled
> > directly to net-next [1] or is there any other (KSZ maintainer's?)
> > tree to which it will be first pulled and then PR will be send to
> > net-next?
> >
> > Thanks in advance for the clarification.
> >
> > Links:
> >
> > [1] -
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/log/
> >
>
> No, there's no other tree than net-next. I see your patch was marked
> as "Changes requested", let me see if I can transition it back to
> "Under review" so that it gains the netdev maintainers' attention
> again:
>
> https://patchwork.kernel.org/project/netdevbpf/cover/[email protected]/
>
> pw-bot: under-review

Thanks!

I've just noticed that there is a WARNING:
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

but then on the newest kernel checkpatch.pl is silent:
./scripts/checkpatch.pl
0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch total: 0
errors, 0 warnings, 0 checks, 277 lines checked

0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch has no
obvious style problems and is ready for submission.

Does the checkpatch for patchwork differs in any way from mainline?


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2023-10-03 13:42:33

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

On Tue, 3 Oct 2023 14:51:06 +0200 Lukasz Majewski wrote:
> I've just noticed that there is a WARNING:
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>
> but then on the newest kernel checkpatch.pl is silent:
> ./scripts/checkpatch.pl
> 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch total: 0
> errors, 0 warnings, 0 checks, 277 lines checked
>
> 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch has no
> obvious style problems and is ready for submission.
>
> Does the checkpatch for patchwork differs in any way from mainline?

We run:

checkpatch with --strict --max-line-length=80

https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh

The "multiple new lines" warning on patch 2 looks legit, no?

2023-10-03 13:51:21

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <[email protected]>:

On Fri, 22 Sep 2023 15:31:03 +0200 you wrote:
> This patch series provides support for HSR HW offloading in KSZ9477
> switch IC.
>
> To test this feature:
> ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
> ip link set dev lan1 up
> ip link set dev lan2 up
> ip a add 192.168.0.1/24 dev hsr0
> ip link set dev hsr0 up
>
> [...]

Here is the summary with links:
- [v6,net-next,1/5] net: dsa: propagate extack to ds->ops->port_hsr_join()
https://git.kernel.org/netdev/net-next/c/fefe5dc4afea
- [v6,net-next,2/5] net: dsa: notify drivers of MAC address changes on user ports
https://git.kernel.org/netdev/net-next/c/6715042cd112
- [v6,net-next,3/5] net: dsa: tag_ksz: Extend ksz9477_xmit() for HSR frame duplication
https://git.kernel.org/netdev/net-next/c/5e5db71a92c5
- [v6,net-next,4/5] net: dsa: microchip: move REG_SW_MAC_ADDR to dev->info->regs[]
https://git.kernel.org/netdev/net-next/c/e5de2ad163e7
- [v6,net-next,5/5] net: dsa: microchip: Enable HSR offloading for KSZ9477
https://git.kernel.org/netdev/net-next/c/2d61298fdd7b

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


2023-10-03 14:16:31

by Lukasz Majewski

[permalink] [raw]
Subject: Re: [PATCH v6 net-next 0/5] net: dsa: hsr: Enable HSR HW offloading for KSZ9477

On Tue, 3 Oct 2023 06:42:13 -0700
Jakub Kicinski <[email protected]> wrote:

> On Tue, 3 Oct 2023 14:51:06 +0200 Lukasz Majewski wrote:
> > I've just noticed that there is a WARNING:
> > https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
> >
> > but then on the newest kernel checkpatch.pl is silent:
> > ./scripts/checkpatch.pl
> > 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch
> > total: 0 errors, 0 warnings, 0 checks, 277 lines checked
> >
> > 0005-net-dsa-microchip-Enable-HSR-offloading-for-KSZ9477.patch has
> > no obvious style problems and is ready for submission.
> >
> > Does the checkpatch for patchwork differs in any way from mainline?
> >
>
> We run:
>
> checkpatch with --strict --max-line-length=80
>
> https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh
>
> The "multiple new lines" warning on patch 2 looks legit, no?

Indeed - the:

'--strict --max-line-length=80'

makes the difference...

If I may ask - why it is added? Or to ask in other way - why the
"vanila" checkpatch is not enough for net-dev ?


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature