* [PATCH] net: hsr: Add support for redbox supervision frames
@ 2021-10-13 7:29 Andreas Oetken
2021-10-13 15:50 ` Jakub Kicinski
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Oetken @ 2021-10-13 7:29 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, netdev, Murali Karicheri
Cc: Andreas Oetken, Andreas Oetken
added support for the redbox supervision frames
as defined in the IEC-62439-3:2018.
Signed-off-by: Andreas Oetken <andreas.oetken@siemens-energy.com>
---
net/hsr/hsr_device.c | 8 ++++----
net/hsr/hsr_forward.c | 12 ++++++------
net/hsr/hsr_framereg.c | 35 +++++++++++++++++++++++++++++++++++
net/hsr/hsr_main.h | 14 ++++++++++----
4 files changed, 55 insertions(+), 14 deletions(-)
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index fdd9c00082a8..b1677b0a9202 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -511,9 +511,9 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
}
spin_unlock_irqrestore(&hsr->seqnr_lock, irqflags);
- hsr_stag->HSR_TLV_type = type;
+ hsr_stag->tlv.HSR_TLV_type = type;
/* TODO: Why 12 in HSRv0? */
- hsr_stag->HSR_TLV_length = hsr->prot_version ?
+ hsr_stag->tlv.HSR_TLV_length = hsr->prot_version ?
sizeof(struct hsr_sup_payload) : 12;
/* Payload: MacAddressA */
@@ -560,8 +560,8 @@ static void send_prp_supervision_frame(struct hsr_port *master,
spin_lock_irqsave(&master->hsr->seqnr_lock, irqflags);
hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
hsr->sup_sequence_nr++;
- hsr_stag->HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
- hsr_stag->HSR_TLV_length = sizeof(struct hsr_sup_payload);
+ hsr_stag->tlv.HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
+ hsr_stag->tlv.HSR_TLV_length = sizeof(struct hsr_sup_payload);
/* Payload: MacAddressA */
hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload));
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index d4d434b9f598..312d6a86c124 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -95,13 +95,13 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
&((struct hsrv0_ethhdr_vlan_sp *)eth_hdr)->hsr_sup;
}
- if (hsr_sup_tag->HSR_TLV_type != HSR_TLV_ANNOUNCE &&
- hsr_sup_tag->HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
- hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
- hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
+ if (hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_ANNOUNCE &&
+ hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
+ hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
+ hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
return false;
- if (hsr_sup_tag->HSR_TLV_length != 12 &&
- hsr_sup_tag->HSR_TLV_length != sizeof(struct hsr_sup_payload))
+ if (hsr_sup_tag->tlv.HSR_TLV_length != 12 &&
+ hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
return false;
return true;
diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index bb1351c38397..e7c6efbc41af 100644
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c
@@ -265,6 +265,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame)
struct hsr_port *port_rcv = frame->port_rcv;
struct hsr_priv *hsr = port_rcv->hsr;
struct hsr_sup_payload *hsr_sp;
+ struct hsr_sup_tlv *hsr_sup_tlv;
struct hsr_node *node_real;
struct sk_buff *skb = NULL;
struct list_head *node_db;
@@ -312,6 +313,40 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame)
/* Node has already been merged */
goto done;
+ /* Leave the first HSR sup payload. */
+ skb_pull(skb, sizeof(struct hsr_sup_payload));
+
+ /* Get second supervision tlv */
+ hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
+ /* And check if it is a redbox mac TLV */
+ if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
+ /* We could stop here after pushing hsr_sup_payload,
+ * or proceed and allow macaddress_B and for redboxes.
+ */
+ /* Sanity check length */
+ if (hsr_sup_tlv->HSR_TLV_length != 6) {
+ skb_push(skb, sizeof(struct hsr_sup_payload));
+ goto done;
+ }
+ /* Leave the second HSR sup tlv. */
+ skb_pull(skb, sizeof(struct hsr_sup_tlv));
+
+ /* Get redbox mac address. */
+ hsr_sp = (struct hsr_sup_payload *)skb->data;
+
+ /* Check if redbox mac and node mac are equal. */
+ if (!ether_addr_equal(node_real->macaddress_A, hsr_sp->macaddress_A)) {
+ /* This is a redbox supervision frame for a VDAN! */
+ /* Push second TLV and payload here */
+ skb_push(skb, sizeof(struct hsr_sup_payload) + sizeof(struct hsr_sup_tlv));
+ goto done;
+ }
+ /* Push second TLV here */
+ skb_push(skb, sizeof(struct hsr_sup_tlv));
+ }
+ /* Push payload here */
+ skb_push(skb, sizeof(struct hsr_sup_payload));
+
ether_addr_copy(node_real->macaddress_B, ethhdr->h_source);
for (i = 0; i < HSR_PT_PORTS; i++) {
if (!node_curr->time_in_stale[i] &&
diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
index bbaef001d55d..fc3bed792ba7 100644
--- a/net/hsr/hsr_main.h
+++ b/net/hsr/hsr_main.h
@@ -43,6 +43,8 @@
#define PRP_TLV_LIFE_CHECK_DD 20
/* PRP V1 life check for Duplicate Accept */
#define PRP_TLV_LIFE_CHECK_DA 21
+/* PRP V1 life redundancy box MAC address */
+#define PRP_TLV_REDBOX_MAC 30
/* HSR Tag.
* As defined in IEC-62439-3:2010, the HSR tag is really { ethertype = 0x88FB,
@@ -95,14 +97,18 @@ struct hsr_vlan_ethhdr {
struct hsr_tag hsr_tag;
} __packed;
+struct hsr_sup_tlv {
+ __u8 HSR_TLV_type;
+ __u8 HSR_TLV_length;
+} __packed;
+
/* HSR/PRP Supervision Frame data types.
* Field names as defined in the IEC:2010 standard for HSR.
*/
struct hsr_sup_tag {
- __be16 path_and_HSR_ver;
- __be16 sequence_nr;
- __u8 HSR_TLV_type;
- __u8 HSR_TLV_length;
+ __be16 path_and_HSR_ver;
+ __be16 sequence_nr;
+ struct hsr_sup_tlv tlv;
} __packed;
struct hsr_sup_payload {
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: hsr: Add support for redbox supervision frames
2021-10-13 7:29 [PATCH] net: hsr: Add support for redbox supervision frames Andreas Oetken
@ 2021-10-13 15:50 ` Jakub Kicinski
2021-10-13 16:13 ` Andreas Oetken
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2021-10-13 15:50 UTC (permalink / raw)
To: Andreas Oetken; +Cc: David S . Miller, netdev, Murali Karicheri, Andreas Oetken
On Wed, 13 Oct 2021 09:29:51 +0200 Andreas Oetken wrote:
> added support for the redbox supervision frames
> as defined in the IEC-62439-3:2018.
>
> Signed-off-by: Andreas Oetken <andreas.oetken@siemens-energy.com>
This does not apply to netdev/net-next.
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index fdd9c00082a8..b1677b0a9202 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -511,9 +511,9 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
> }
> spin_unlock_irqrestore(&hsr->seqnr_lock, irqflags);
>
> - hsr_stag->HSR_TLV_type = type;
> + hsr_stag->tlv.HSR_TLV_type = type;
> /* TODO: Why 12 in HSRv0? */
> - hsr_stag->HSR_TLV_length = hsr->prot_version ?
> + hsr_stag->tlv.HSR_TLV_length = hsr->prot_version ?
> sizeof(struct hsr_sup_payload) : 12;
>
> /* Payload: MacAddressA */
> @@ -560,8 +560,8 @@ static void send_prp_supervision_frame(struct hsr_port *master,
> spin_lock_irqsave(&master->hsr->seqnr_lock, irqflags);
> hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
> hsr->sup_sequence_nr++;
> - hsr_stag->HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
> - hsr_stag->HSR_TLV_length = sizeof(struct hsr_sup_payload);
> + hsr_stag->tlv.HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
> + hsr_stag->tlv.HSR_TLV_length = sizeof(struct hsr_sup_payload);
>
> /* Payload: MacAddressA */
> hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload));
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index d4d434b9f598..312d6a86c124 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -95,13 +95,13 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
> &((struct hsrv0_ethhdr_vlan_sp *)eth_hdr)->hsr_sup;
> }
>
> - if (hsr_sup_tag->HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> - hsr_sup_tag->HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
> - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> + if (hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> + hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
> + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> return false;
> - if (hsr_sup_tag->HSR_TLV_length != 12 &&
> - hsr_sup_tag->HSR_TLV_length != sizeof(struct hsr_sup_payload))
> + if (hsr_sup_tag->tlv.HSR_TLV_length != 12 &&
> + hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct hsr_sup_payload))
> return false;
>
> return true;
> diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
> index bb1351c38397..e7c6efbc41af 100644
> --- a/net/hsr/hsr_framereg.c
> +++ b/net/hsr/hsr_framereg.c
> @@ -265,6 +265,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame)
> struct hsr_port *port_rcv = frame->port_rcv;
> struct hsr_priv *hsr = port_rcv->hsr;
> struct hsr_sup_payload *hsr_sp;
> + struct hsr_sup_tlv *hsr_sup_tlv;
> struct hsr_node *node_real;
> struct sk_buff *skb = NULL;
> struct list_head *node_db;
> @@ -312,6 +313,40 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame)
> /* Node has already been merged */
> goto done;
>
> + /* Leave the first HSR sup payload. */
> + skb_pull(skb, sizeof(struct hsr_sup_payload));
> +
> + /* Get second supervision tlv */
> + hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
> + /* And check if it is a redbox mac TLV */
> + if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> + /* We could stop here after pushing hsr_sup_payload,
> + * or proceed and allow macaddress_B and for redboxes.
> + */
> + /* Sanity check length */
> + if (hsr_sup_tlv->HSR_TLV_length != 6) {
> + skb_push(skb, sizeof(struct hsr_sup_payload));
> + goto done;
> + }
> + /* Leave the second HSR sup tlv. */
> + skb_pull(skb, sizeof(struct hsr_sup_tlv));
> +
> + /* Get redbox mac address. */
> + hsr_sp = (struct hsr_sup_payload *)skb->data;
> +
> + /* Check if redbox mac and node mac are equal. */
> + if (!ether_addr_equal(node_real->macaddress_A, hsr_sp->macaddress_A)) {
> + /* This is a redbox supervision frame for a VDAN! */
> + /* Push second TLV and payload here */
> + skb_push(skb, sizeof(struct hsr_sup_payload) + sizeof(struct hsr_sup_tlv));
> + goto done;
> + }
> + /* Push second TLV here */
> + skb_push(skb, sizeof(struct hsr_sup_tlv));
> + }
> + /* Push payload here */
> + skb_push(skb, sizeof(struct hsr_sup_payload));
Is this code path handling frames from the network or user space?
Does it need input checking?
> ether_addr_copy(node_real->macaddress_B, ethhdr->h_source);
> for (i = 0; i < HSR_PT_PORTS; i++) {
> if (!node_curr->time_in_stale[i] &&
> diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
> index bbaef001d55d..fc3bed792ba7 100644
> --- a/net/hsr/hsr_main.h
> +++ b/net/hsr/hsr_main.h
> @@ -43,6 +43,8 @@
> #define PRP_TLV_LIFE_CHECK_DD 20
> /* PRP V1 life check for Duplicate Accept */
> #define PRP_TLV_LIFE_CHECK_DA 21
> +/* PRP V1 life redundancy box MAC address */
> +#define PRP_TLV_REDBOX_MAC 30
>
> /* HSR Tag.
> * As defined in IEC-62439-3:2010, the HSR tag is really { ethertype = 0x88FB,
> @@ -95,14 +97,18 @@ struct hsr_vlan_ethhdr {
> struct hsr_tag hsr_tag;
> } __packed;
>
> +struct hsr_sup_tlv {
> + __u8 HSR_TLV_type;
> + __u8 HSR_TLV_length;
u8, __u8 is for uAPI headers, which this is not.
> +} __packed;
There's no need to pack structs which only have members of size 1.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: hsr: Add support for redbox supervision frames
2021-10-13 15:50 ` Jakub Kicinski
@ 2021-10-13 16:13 ` Andreas Oetken
2021-10-13 16:37 ` Jakub Kicinski
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Oetken @ 2021-10-13 16:13 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David S . Miller, netdev, Murali Karicheri
Thank you Jakub for your notes.
Am Mittwoch, dem 13.10.2021 um 08:50 -0700 schrieb Jakub Kicinski:
> On Wed, 13 Oct 2021 09:29:51 +0200 Andreas Oetken wrote:
> > added support for the redbox supervision frames
> > as defined in the IEC-62439-3:2018.
> >
> > Signed-off-by: Andreas Oetken <andreas.oetken@siemens-energy.com>
>
> This does not apply to netdev/net-next.
Sorry, I will not include it next time.
>
> > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > index fdd9c00082a8..b1677b0a9202 100644
> > --- a/net/hsr/hsr_device.c
> > +++ b/net/hsr/hsr_device.c
> > @@ -511,9 +511,9 @@ static void send_hsr_supervision_frame(struct
> > hsr_port *master,
> > }
> > spin_unlock_irqrestore(&hsr->seqnr_lock, irqflags);
> >
> > - hsr_stag->HSR_TLV_type = type;
> > + hsr_stag->tlv.HSR_TLV_type = type;
> > /* TODO: Why 12 in HSRv0? */
> > - hsr_stag->HSR_TLV_length = hsr->prot_version ?
> > + hsr_stag->tlv.HSR_TLV_length = hsr->prot_version ?
> > sizeof(struct hsr_sup_payload) :
> > 12;
> >
> > /* Payload: MacAddressA */
> > @@ -560,8 +560,8 @@ static void send_prp_supervision_frame(struct
> > hsr_port *master,
> > spin_lock_irqsave(&master->hsr->seqnr_lock, irqflags);
> > hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
> > hsr->sup_sequence_nr++;
> > - hsr_stag->HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
> > - hsr_stag->HSR_TLV_length = sizeof(struct hsr_sup_payload);
> > + hsr_stag->tlv.HSR_TLV_type = PRP_TLV_LIFE_CHECK_DD;
> > + hsr_stag->tlv.HSR_TLV_length = sizeof(struct
> > hsr_sup_payload);
> >
> > /* Payload: MacAddressA */
> > hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload));
> > diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> > index d4d434b9f598..312d6a86c124 100644
> > --- a/net/hsr/hsr_forward.c
> > +++ b/net/hsr/hsr_forward.c
> > @@ -95,13 +95,13 @@ static bool is_supervision_frame(struct
> > hsr_priv *hsr, struct sk_buff *skb)
> > &((struct hsrv0_ethhdr_vlan_sp *)eth_hdr)-
> > >hsr_sup;
> > }
> >
> > - if (hsr_sup_tag->HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> > - hsr_sup_tag->HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> > - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
> > - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> > + if (hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> > + hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> > + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD
> > &&
> > + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> > return false;
> > - if (hsr_sup_tag->HSR_TLV_length != 12 &&
> > - hsr_sup_tag->HSR_TLV_length != sizeof(struct
> > hsr_sup_payload))
> > + if (hsr_sup_tag->tlv.HSR_TLV_length != 12 &&
> > + hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct
> > hsr_sup_payload))
> > return false;
> >
> > return true;
> > diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
> > index bb1351c38397..e7c6efbc41af 100644
> > --- a/net/hsr/hsr_framereg.c
> > +++ b/net/hsr/hsr_framereg.c
> > @@ -265,6 +265,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info
> > *frame)
> > struct hsr_port *port_rcv = frame->port_rcv;
> > struct hsr_priv *hsr = port_rcv->hsr;
> > struct hsr_sup_payload *hsr_sp;
> > + struct hsr_sup_tlv *hsr_sup_tlv;
> > struct hsr_node *node_real;
> > struct sk_buff *skb = NULL;
> > struct list_head *node_db;
> > @@ -312,6 +313,40 @@ void hsr_handle_sup_frame(struct
> > hsr_frame_info *frame)
> > /* Node has already been merged */
> > goto done;
> >
> > + /* Leave the first HSR sup payload. */
> > + skb_pull(skb, sizeof(struct hsr_sup_payload));
> > +
> > + /* Get second supervision tlv */
> > + hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
> > + /* And check if it is a redbox mac TLV */
> > + if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> > + /* We could stop here after pushing
> > hsr_sup_payload,
> > + * or proceed and allow macaddress_B and for
> > redboxes.
> > + */
> > + /* Sanity check length */
> > + if (hsr_sup_tlv->HSR_TLV_length != 6) {
> > + skb_push(skb, sizeof(struct
> > hsr_sup_payload));
> > + goto done;
> > + }
> > + /* Leave the second HSR sup tlv. */
> > + skb_pull(skb, sizeof(struct hsr_sup_tlv));
> > +
> > + /* Get redbox mac address. */
> > + hsr_sp = (struct hsr_sup_payload *)skb->data;
> > +
> > + /* Check if redbox mac and node mac are equal. */
> > + if (!ether_addr_equal(node_real->macaddress_A,
> > hsr_sp->macaddress_A)) {
> > + /* This is a redbox supervision frame for a
> > VDAN! */
> > + /* Push second TLV and payload here */
> > + skb_push(skb, sizeof(struct
> > hsr_sup_payload) + sizeof(struct hsr_sup_tlv));
> > + goto done;
> > + }
> > + /* Push second TLV here */
> > + skb_push(skb, sizeof(struct hsr_sup_tlv));
> > + }
> > + /* Push payload here */
> > + skb_push(skb, sizeof(struct hsr_sup_payload));
>
> Is this code path handling frames from the network or user space?
> Does it need input checking?
This code path is handling frames from the network. The supervision
frames are broadcasted by each device. What type of additional input
checking are you thinking of? I think I should check skb->len before
pulling/accessing right?
>
> > ether_addr_copy(node_real->macaddress_B, ethhdr->h_source);
> > for (i = 0; i < HSR_PT_PORTS; i++) {
> > if (!node_curr->time_in_stale[i] &&
> > diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
> > index bbaef001d55d..fc3bed792ba7 100644
> > --- a/net/hsr/hsr_main.h
> > +++ b/net/hsr/hsr_main.h
> > @@ -43,6 +43,8 @@
> > #define PRP_TLV_LIFE_CHECK_DD 20
> > /* PRP V1 life check for Duplicate Accept */
> > #define PRP_TLV_LIFE_CHECK_DA 21
> > +/* PRP V1 life redundancy box MAC address */
> > +#define PRP_TLV_REDBOX_MAC 30
> >
> > /* HSR Tag.
> > * As defined in IEC-62439-3:2010, the HSR tag is really {
> > ethertype = 0x88FB,
> > @@ -95,14 +97,18 @@ struct hsr_vlan_ethhdr {
> > struct hsr_tag hsr_tag;
> > } __packed;
> >
> > +struct hsr_sup_tlv {
> > + __u8 HSR_TLV_type;
> > + __u8 HSR_TLV_length;
>
> u8, __u8 is for uAPI headers, which this is not.
Changed it.
>
> > +} __packed;
>
> There's no need to pack structs which only have members of size 1.
>
Changed it too.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: hsr: Add support for redbox supervision frames
2021-10-13 16:13 ` Andreas Oetken
@ 2021-10-13 16:37 ` Jakub Kicinski
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2021-10-13 16:37 UTC (permalink / raw)
To: Andreas Oetken; +Cc: David S . Miller, netdev, Murali Karicheri
On Wed, 13 Oct 2021 18:13:48 +0200 Andreas Oetken wrote:
> Am Mittwoch, dem 13.10.2021 um 08:50 -0700 schrieb Jakub Kicinski:
> > On Wed, 13 Oct 2021 09:29:51 +0200 Andreas Oetken wrote:
> > > added support for the redbox supervision frames
> > > as defined in the IEC-62439-3:2018.
> > >
> > > Signed-off-by: Andreas Oetken <andreas.oetken@siemens-energy.com>
> >
> > This does not apply to netdev/net-next.
> Sorry, I will not include it next time.
To be clear - please rebase on that tree, and also include
[PATCH net-next] in the subject.
> > >
> > > - if (hsr_sup_tag->HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> > > - hsr_sup_tag->HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> > > - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD &&
> > > - hsr_sup_tag->HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> > > + if (hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_ANNOUNCE &&
> > > + hsr_sup_tag->tlv.HSR_TLV_type != HSR_TLV_LIFE_CHECK &&
> > > + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DD
> > > &&
> > > + hsr_sup_tag->tlv.HSR_TLV_type != PRP_TLV_LIFE_CHECK_DA)
> > > return false;
> > > - if (hsr_sup_tag->HSR_TLV_length != 12 &&
> > > - hsr_sup_tag->HSR_TLV_length != sizeof(struct
> > > hsr_sup_payload))
> > > + if (hsr_sup_tag->tlv.HSR_TLV_length != 12 &&
> > > + hsr_sup_tag->tlv.HSR_TLV_length != sizeof(struct
> > > hsr_sup_payload))
> > > return false;
> > >
> > > return true;
> > > diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
> > > index bb1351c38397..e7c6efbc41af 100644
> > > --- a/net/hsr/hsr_framereg.c
> > > +++ b/net/hsr/hsr_framereg.c
> > > @@ -265,6 +265,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info
> > > *frame)
> > > struct hsr_port *port_rcv = frame->port_rcv;
> > > struct hsr_priv *hsr = port_rcv->hsr;
> > > struct hsr_sup_payload *hsr_sp;
> > > + struct hsr_sup_tlv *hsr_sup_tlv;
> > > struct hsr_node *node_real;
> > > struct sk_buff *skb = NULL;
> > > struct list_head *node_db;
> > > @@ -312,6 +313,40 @@ void hsr_handle_sup_frame(struct
> > > hsr_frame_info *frame)
> > > /* Node has already been merged */
> > > goto done;
> > >
> > > + /* Leave the first HSR sup payload. */
> > > + skb_pull(skb, sizeof(struct hsr_sup_payload));
> > > +
> > > + /* Get second supervision tlv */
> > > + hsr_sup_tlv = (struct hsr_sup_tlv *)skb->data;
> > > + /* And check if it is a redbox mac TLV */
> > > + if (hsr_sup_tlv->HSR_TLV_type == PRP_TLV_REDBOX_MAC) {
> > > + /* We could stop here after pushing
> > > hsr_sup_payload,
> > > + * or proceed and allow macaddress_B and for
> > > redboxes.
> > > + */
> > > + /* Sanity check length */
> > > + if (hsr_sup_tlv->HSR_TLV_length != 6) {
> > > + skb_push(skb, sizeof(struct
> > > hsr_sup_payload));
> > > + goto done;
> > > + }
> > > + /* Leave the second HSR sup tlv. */
> > > + skb_pull(skb, sizeof(struct hsr_sup_tlv));
> > > +
> > > + /* Get redbox mac address. */
> > > + hsr_sp = (struct hsr_sup_payload *)skb->data;
> > > +
> > > + /* Check if redbox mac and node mac are equal. */
> > > + if (!ether_addr_equal(node_real->macaddress_A,
> > > hsr_sp->macaddress_A)) {
> > > + /* This is a redbox supervision frame for a
> > > VDAN! */
> > > + /* Push second TLV and payload here */
> > > + skb_push(skb, sizeof(struct
> > > hsr_sup_payload) + sizeof(struct hsr_sup_tlv));
> > > + goto done;
> > > + }
> > > + /* Push second TLV here */
> > > + skb_push(skb, sizeof(struct hsr_sup_tlv));
> > > + }
> > > + /* Push payload here */
> > > + skb_push(skb, sizeof(struct hsr_sup_payload));
> >
> > Is this code path handling frames from the network or user space?
> > Does it need input checking?
> This code path is handling frames from the network. The supervision
> frames are broadcasted by each device. What type of additional input
> checking are you thinking of? I think I should check skb->len before
> pulling/accessing right?
Yes, skb->len would be the start, but skb can also have fragments, so
skb->len > N does not guarantee that N bytes are accessible directly
via skb->data. I think that calling pskb_may_pull() would be the best
fit for your use.
I'm not sure why this function doesn't do it already, perhaps there
is some guarantee I'm missing.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-13 16:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 7:29 [PATCH] net: hsr: Add support for redbox supervision frames Andreas Oetken
2021-10-13 15:50 ` Jakub Kicinski
2021-10-13 16:13 ` Andreas Oetken
2021-10-13 16:37 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.