* [PATCH] ipv6: sr: update the struct ipv6_sr_hdr
@ 2017-11-12 20:37 Ahmed Abdelsalam
2017-11-14 12:37 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Ahmed Abdelsalam @ 2017-11-12 20:37 UTC (permalink / raw)
To: davem, david.lebrun; +Cc: amsalam20, netdev
The IPv6 Segment Routing Header (SRH) format has been updated srating
from revision 6 of the SRH ietf draft. The update includes the following
SRH fields
(1) The "First Segment" field changed to be "Last Entry" which contains
the index, in the Segment List, of the last element of the Segment List.
(2) The 16 bit "reserved" field now is used as a "tag" which tags a packet
as part of a class or group of packets, e.g.,packets sharing the same
set of properties.
This patch updates the struct ipv6_sr_hdr, so it complies with the updated
SRH draft. It also update the different parts of the kernel that were
using the old fields names.
Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
---
This patch is tested by re-compiling the whole kernel after the changes.
include/uapi/linux/seg6.h | 4 ++--
net/ipv6/exthdrs.c | 2 +-
net/ipv6/seg6.c | 4 ++--
net/ipv6/seg6_hmac.c | 14 +++++++-------
net/ipv6/seg6_iptunnel.c | 4 ++--
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/uapi/linux/seg6.h b/include/uapi/linux/seg6.h
index 2f6fb0d..3f4b3ab 100644
--- a/include/uapi/linux/seg6.h
+++ b/include/uapi/linux/seg6.h
@@ -26,9 +26,9 @@ struct ipv6_sr_hdr {
__u8 hdrlen;
__u8 type;
__u8 segments_left;
- __u8 first_segment;
+ __u8 last_entry;
__u8 flags;
- __u16 reserved;
+ __u16 tag;
struct in6_addr segments[0];
};
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 83bd757..d53af71 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -918,7 +918,7 @@ static void ipv6_push_rthdr4(struct sk_buff *skb, u8 *proto,
sr_phdr = skb_push(skb, plen);
memcpy(sr_phdr, sr_ihdr, sizeof(struct ipv6_sr_hdr));
- hops = sr_ihdr->first_segment + 1;
+ hops = sr_ihdr->last_entry + 1;
memcpy(sr_phdr->segments + 1, sr_ihdr->segments + 1,
(hops - 1) * sizeof(struct in6_addr));
diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
index c814077..3d5279d 100644
--- a/net/ipv6/seg6.c
+++ b/net/ipv6/seg6.c
@@ -40,10 +40,10 @@ bool seg6_validate_srh(struct ipv6_sr_hdr *srh, int len)
if (((srh->hdrlen + 1) << 3) != len)
return false;
- if (srh->segments_left > srh->first_segment)
+ if (srh->segments_left > srh->last_entry)
return false;
- tlv_offset = sizeof(*srh) + ((srh->first_segment + 1) << 4);
+ tlv_offset = sizeof(*srh) + ((srh->last_entry + 1) << 4);
trailing = len - tlv_offset;
if (trailing < 0)
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index 33fb35c..5107ebb 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -91,7 +91,7 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
{
struct sr6_tlv_hmac *tlv;
- if (srh->hdrlen < (srh->first_segment + 1) * 2 + 5)
+ if (srh->hdrlen < (srh->last_entry + 1) * 2 + 5)
return NULL;
if (!sr_has_hmac(srh))
@@ -175,8 +175,8 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr,
* hash function (RadioGatun) with up to 1216 bits
*/
- /* saddr(16) + first_seg(1) + flags(1) + keyid(4) + seglist(16n) */
- plen = 16 + 1 + 1 + 4 + (hdr->first_segment + 1) * 16;
+ /* saddr(16) + last_entry(1) + flags(1) + keyid(4) + seglist(16n) */
+ plen = 16 + 1 + 1 + 4 + (hdr->last_entry + 1) * 16;
/* this limit allows for 14 segments */
if (plen >= SEG6_HMAC_RING_SIZE)
@@ -186,7 +186,7 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr,
* as follows, in order:
*
* 1. Source IPv6 address (128 bits)
- * 2. first_segment value (8 bits)
+ * 2. last_entry value (8 bits)
* 3. Flags (8 bits)
* 4. HMAC Key ID (32 bits)
* 5. All segments in the segments list (n * 128 bits)
@@ -200,8 +200,8 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr,
memcpy(off, saddr, 16);
off += 16;
- /* first_segment value */
- *off++ = hdr->first_segment;
+ /* last_entry value */
+ *off++ = hdr->last_entry;
/* flags */
*off++ = hdr->flags;
@@ -211,7 +211,7 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr,
off += 4;
/* all segments in the list */
- for (i = 0; i < hdr->first_segment + 1; i++) {
+ for (i = 0; i < hdr->last_entry + 1; i++) {
memcpy(off, hdr->segments + i, 16);
off += 16;
}
diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index bd6cc68..fc9813e 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -133,7 +133,7 @@ int seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh, int proto)
isrh->nexthdr = proto;
- hdr->daddr = isrh->segments[isrh->first_segment];
+ hdr->daddr = isrh->segments[isrh->last_entry];
set_tun_src(net, skb->dev, &hdr->daddr, &hdr->saddr);
#ifdef CONFIG_IPV6_SEG6_HMAC
@@ -184,7 +184,7 @@ int seg6_do_srh_inline(struct sk_buff *skb, struct ipv6_sr_hdr *osrh)
hdr->nexthdr = NEXTHDR_ROUTING;
isrh->segments[0] = hdr->daddr;
- hdr->daddr = isrh->segments[isrh->first_segment];
+ hdr->daddr = isrh->segments[isrh->last_entry];
#ifdef CONFIG_IPV6_SEG6_HMAC
if (sr_has_hmac(isrh)) {
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] ipv6: sr: update the struct ipv6_sr_hdr
2017-11-12 20:37 [PATCH] ipv6: sr: update the struct ipv6_sr_hdr Ahmed Abdelsalam
@ 2017-11-14 12:37 ` David Miller
2017-11-14 14:14 ` Edward Cree
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2017-11-14 12:37 UTC (permalink / raw)
To: amsalam20; +Cc: david.lebrun, netdev
From: Ahmed Abdelsalam <amsalam20@gmail.com>
Date: Sun, 12 Nov 2017 21:37:01 +0100
> diff --git a/include/uapi/linux/seg6.h b/include/uapi/linux/seg6.h
> index 2f6fb0d..3f4b3ab 100644
> --- a/include/uapi/linux/seg6.h
> +++ b/include/uapi/linux/seg6.h
> @@ -26,9 +26,9 @@ struct ipv6_sr_hdr {
> __u8 hdrlen;
> __u8 type;
> __u8 segments_left;
> - __u8 first_segment;
> + __u8 last_entry;
This is user ABI and cannot be changed.
Sorry, folks should have considered these issues when the SR
changes were submitted. This field must keep the name 'first_segment'
forever.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ipv6: sr: update the struct ipv6_sr_hdr
2017-11-14 12:37 ` David Miller
@ 2017-11-14 14:14 ` Edward Cree
2017-11-14 14:31 ` Ahmed Abdelsalam
2017-11-15 0:54 ` David Miller
0 siblings, 2 replies; 8+ messages in thread
From: Edward Cree @ 2017-11-14 14:14 UTC (permalink / raw)
To: David Miller, amsalam20; +Cc: david.lebrun, netdev
On 14/11/17 12:37, David Miller wrote:
> From: Ahmed Abdelsalam <amsalam20@gmail.com>
> Date: Sun, 12 Nov 2017 21:37:01 +0100
>
>> diff --git a/include/uapi/linux/seg6.h b/include/uapi/linux/seg6.h
>> index 2f6fb0d..3f4b3ab 100644
>> --- a/include/uapi/linux/seg6.h
>> +++ b/include/uapi/linux/seg6.h
>> @@ -26,9 +26,9 @@ struct ipv6_sr_hdr {
>> __u8 hdrlen;
>> __u8 type;
>> __u8 segments_left;
>> - __u8 first_segment;
>> + __u8 last_entry;
>
> This is user ABI and cannot be changed.
>
> Sorry, folks should have considered these issues when the SR
> changes were submitted. This field must keep the name 'first_segment'
> forever.
Surely renaming struct fields only changes the API, not the ABI?
Binaries compiled against the old struct definition will still behave the
same (AFAICT the patch doesn't change how these fields are used), while
sources being recompiled (so they care about the name change) can be
changed.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ipv6: sr: update the struct ipv6_sr_hdr
2017-11-14 14:14 ` Edward Cree
@ 2017-11-14 14:31 ` Ahmed Abdelsalam
2017-11-15 0:55 ` David Miller
2017-11-15 0:54 ` David Miller
1 sibling, 1 reply; 8+ messages in thread
From: Ahmed Abdelsalam @ 2017-11-14 14:31 UTC (permalink / raw)
To: Edward Cree; +Cc: David Miller, david.lebrun, netdev
On Tue, 14 Nov 2017 14:14:01 +0000
Edward Cree <ecree@solarflare.com> wrote:
> On 14/11/17 12:37, David Miller wrote:
> > From: Ahmed Abdelsalam <amsalam20@gmail.com>
> > Date: Sun, 12 Nov 2017 21:37:01 +0100
> >
> >> diff --git a/include/uapi/linux/seg6.h b/include/uapi/linux/seg6.h
> >> index 2f6fb0d..3f4b3ab 100644
> >> --- a/include/uapi/linux/seg6.h
> >> +++ b/include/uapi/linux/seg6.h
> >> @@ -26,9 +26,9 @@ struct ipv6_sr_hdr {
> >> __u8 hdrlen;
> >> __u8 type;
> >> __u8 segments_left;
> >> - __u8 first_segment;
> >> + __u8 last_entry;
> >
> > This is user ABI and cannot be changed.
> >
> > Sorry, folks should have considered these issues when the SR
> > changes were submitted. This field must keep the name 'first_segment'
> > forever.
>
> Surely renaming struct fields only changes the API, not the ABI?
> Binaries compiled against the old struct definition will still behave the
> same (AFAICT the patch doesn't change how these fields are used), while
> sources being recompiled (so they care about the name change) can be
> changed.
Yes Exactly,
What I meant by the patch is to change just the field name, but the logic still the same.
Also it will not make sense to have the field name differnent from the draft.
--
Ahmed
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] ipv6: sr: update the struct ipv6_sr_hdr
2017-11-14 14:31 ` Ahmed Abdelsalam
@ 2017-11-15 0:55 ` David Miller
2017-11-15 11:42 ` Ahmed Abdelsalam
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2017-11-15 0:55 UTC (permalink / raw)
To: amsalam20; +Cc: ecree, david.lebrun, netdev
From: Ahmed Abdelsalam <amsalam20@gmail.com>
Date: Tue, 14 Nov 2017 15:31:48 +0100
> Also it will not make sense to have the field name differnent from the draft.
That is the danger of defining user facing things against a draft which
is constantly changing.
Sorry, we are stuck with the current name.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: update the struct ipv6_sr_hdr
2017-11-15 0:55 ` David Miller
@ 2017-11-15 11:42 ` Ahmed Abdelsalam
2017-11-15 13:37 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Ahmed Abdelsalam @ 2017-11-15 11:42 UTC (permalink / raw)
To: David Miller; +Cc: ecree, david.lebrun, netdev
On Wed, 15 Nov 2017 09:55:32 +0900 (KST)
David Miller <davem@davemloft.net> wrote:
> From: Ahmed Abdelsalam <amsalam20@gmail.com>
> Date: Tue, 14 Nov 2017 15:31:48 +0100
>
> > Also it will not make sense to have the field name differnent from the draft.
>
> That is the danger of defining user facing things against a draft which
> is constantly changing.
>
I totally understand your point of keeping the user API unchanged.
> Sorry, we are stuck with the current name.
For the time being, Can i submit another patch updating the "reserved" bits to be "tag" and just adding a comment next the "first_segment".
In the comment we can mention that this represent the "last_entry" field of the Segment Routing Header (SRH).
--
Ahmed
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ipv6: sr: update the struct ipv6_sr_hdr
2017-11-14 14:14 ` Edward Cree
2017-11-14 14:31 ` Ahmed Abdelsalam
@ 2017-11-15 0:54 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2017-11-15 0:54 UTC (permalink / raw)
To: ecree; +Cc: amsalam20, david.lebrun, netdev
From: Edward Cree <ecree@solarflare.com>
Date: Tue, 14 Nov 2017 14:14:01 +0000
> On 14/11/17 12:37, David Miller wrote:
>> From: Ahmed Abdelsalam <amsalam20@gmail.com>
>> Date: Sun, 12 Nov 2017 21:37:01 +0100
>>
>>> diff --git a/include/uapi/linux/seg6.h b/include/uapi/linux/seg6.h
>>> index 2f6fb0d..3f4b3ab 100644
>>> --- a/include/uapi/linux/seg6.h
>>> +++ b/include/uapi/linux/seg6.h
>>> @@ -26,9 +26,9 @@ struct ipv6_sr_hdr {
>>> __u8 hdrlen;
>>> __u8 type;
>>> __u8 segments_left;
>>> - __u8 first_segment;
>>> + __u8 last_entry;
>>
>> This is user ABI and cannot be changed.
>>
>> Sorry, folks should have considered these issues when the SR
>> changes were submitted. This field must keep the name 'first_segment'
>> forever.
>
> Surely renaming struct fields only changes the API, not the ABI?
> Binaries compiled against the old struct definition will still behave the
> same (AFAICT the patch doesn't change how these fields are used), while
> sources being recompiled (so they care about the name change) can be
> changed.
Yes, it will stop existing applications from compiling and we can't
do that.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-11-15 13:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-12 20:37 [PATCH] ipv6: sr: update the struct ipv6_sr_hdr Ahmed Abdelsalam
2017-11-14 12:37 ` David Miller
2017-11-14 14:14 ` Edward Cree
2017-11-14 14:31 ` Ahmed Abdelsalam
2017-11-15 0:55 ` David Miller
2017-11-15 11:42 ` Ahmed Abdelsalam
2017-11-15 13:37 ` David Miller
2017-11-15 0:54 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).