* [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter [not found] <022D9517-4804-47BB-B789-CCF25F25C1ED@me.com> 2014-03-18 6:12 ` behanw @ 2014-03-18 6:12 ` behanw 0 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-18 6:12 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vin√≠cius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..f83a519 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,29 +5,50 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + char data[0]; \ + } *tbl; \ + struct type##_standard *entries; \ + struct type##_error *term; \ + size_t entries_offset = (-sizeof(tbl->repl)) & \ + (__alignof__(*entries)-1); \ + size_t entries_end = entries_offset + nhooks * sizeof(*entries); \ + size_t term_offset = entries_end + \ + (-sizeof(tbl->repl) - entries_end) & (__alignof__(*term)-1); \ + size_t term_end = term_offset + sizeof(*term); \ + size_t data_sz = term_end + \ + (-sizeof(tbl->repl) - term_end) & (__alignof__(tbl->repl)-1); \ + size_t tbl_sz = sizeof(tbl->repl) + data_sz; \ + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + entries = (struct type##_standard *)&tbl->data[entries_offset]; \ + term = (struct type##_error *)&tbl->data[term_offset]; \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ tbl->repl.hook_entry[hooknum] = bytes; \ tbl->repl.underflow[hooknum] = bytes; \ - tbl->entries[i++] = (struct type##_standard) \ + entries[i++] = (struct type##_standard) \ typ2##_STANDARD_INIT(NF_ACCEPT); \ bytes += sizeof(struct type##_standard); \ } \ -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-18 6:12 ` behanw 0 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-18 6:12 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..f83a519 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,29 +5,50 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + char data[0]; \ + } *tbl; \ + struct type##_standard *entries; \ + struct type##_error *term; \ + size_t entries_offset = (-sizeof(tbl->repl)) & \ + (__alignof__(*entries)-1); \ + size_t entries_end = entries_offset + nhooks * sizeof(*entries); \ + size_t term_offset = entries_end + \ + (-sizeof(tbl->repl) - entries_end) & (__alignof__(*term)-1); \ + size_t term_end = term_offset + sizeof(*term); \ + size_t data_sz = term_end + \ + (-sizeof(tbl->repl) - term_end) & (__alignof__(tbl->repl)-1); \ + size_t tbl_sz = sizeof(tbl->repl) + data_sz; \ + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + entries = (struct type##_standard *)&tbl->data[entries_offset]; \ + term = (struct type##_error *)&tbl->data[term_offset]; \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ tbl->repl.hook_entry[hooknum] = bytes; \ tbl->repl.underflow[hooknum] = bytes; \ - tbl->entries[i++] = (struct type##_standard) \ + entries[i++] = (struct type##_standard) \ typ2##_STANDARD_INIT(NF_ACCEPT); \ bytes += sizeof(struct type##_standard); \ } \ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-18 6:12 ` behanw 0 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-18 6:12 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..f83a519 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,29 +5,50 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + char data[0]; \ + } *tbl; \ + struct type##_standard *entries; \ + struct type##_error *term; \ + size_t entries_offset = (-sizeof(tbl->repl)) & \ + (__alignof__(*entries)-1); \ + size_t entries_end = entries_offset + nhooks * sizeof(*entries); \ + size_t term_offset = entries_end + \ + (-sizeof(tbl->repl) - entries_end) & (__alignof__(*term)-1); \ + size_t term_end = term_offset + sizeof(*term); \ + size_t data_sz = term_end + \ + (-sizeof(tbl->repl) - term_end) & (__alignof__(tbl->repl)-1); \ + size_t tbl_sz = sizeof(tbl->repl) + data_sz; \ + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + entries = (struct type##_standard *)&tbl->data[entries_offset]; \ + term = (struct type##_error *)&tbl->data[term_offset]; \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ tbl->repl.hook_entry[hooknum] = bytes; \ tbl->repl.underflow[hooknum] = bytes; \ - tbl->entries[i++] = (struct type##_standard) \ + entries[i++] = (struct type##_standard) \ typ2##_STANDARD_INIT(NF_ACCEPT); \ bytes += sizeof(struct type##_standard); \ } \ -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-18 6:12 ` behanw @ 2014-03-18 9:41 ` David Laight -1 siblings, 0 replies; 21+ messages in thread From: David Laight @ 2014-03-18 9:41 UTC (permalink / raw) To: 'behanw@converseincode.com', davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti From: behanw@converseincode.com > From: Mark Charlebois <charlebm@gmail.com> > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > xt_repldata.h with a C99 compliant flexible array member and then calculated > offsets to the other struct members. These other members aren't referenced by > name in this code, however this patch maintains the same memory layout and > padding as was previously accomplished using VLAIS. > > Had the original structure been ordered differently, with the entries VLA at > the end, then it could have been a flexible member, and this patch would have > been a lot simpler. However since the data stored in this structure is > ultimately exported to userspace, the order of this structure can't be changed. Why not just remove the last element and allocate space for it after the structure? That would reduce the complexity of the patch and the unreadability of the new code. I realise that the alignment of type##_error is 'tricky' to determine. David ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-18 9:41 ` David Laight 0 siblings, 0 replies; 21+ messages in thread From: David Laight @ 2014-03-18 9:41 UTC (permalink / raw) To: 'behanw@converseincode.com', davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1176 bytes --] From: behanw@converseincode.com > From: Mark Charlebois <charlebm@gmail.com> > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > xt_repldata.h with a C99 compliant flexible array member and then calculated > offsets to the other struct members. These other members aren't referenced by > name in this code, however this patch maintains the same memory layout and > padding as was previously accomplished using VLAIS. > > Had the original structure been ordered differently, with the entries VLA at > the end, then it could have been a flexible member, and this patch would have > been a lot simpler. However since the data stored in this structure is > ultimately exported to userspace, the order of this structure can't be changed. Why not just remove the last element and allocate space for it after the structure? That would reduce the complexity of the patch and the unreadability of the new code. I realise that the alignment of type##_error is 'tricky' to determine. David ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-18 9:41 ` David Laight (?) @ 2014-03-18 14:47 ` Behan Webster 2014-03-18 15:24 ` David Laight -1 siblings, 1 reply; 21+ messages in thread From: Behan Webster @ 2014-03-18 14:47 UTC (permalink / raw) To: David Laight, davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti On 03/18/14 02:41, David Laight wrote: > From: behanw@converseincode.com >> From: Mark Charlebois <charlebm@gmail.com> >> >> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in >> xt_repldata.h with a C99 compliant flexible array member and then calculated >> offsets to the other struct members. These other members aren't referenced by >> name in this code, however this patch maintains the same memory layout and >> padding as was previously accomplished using VLAIS. >> >> Had the original structure been ordered differently, with the entries VLA at >> the end, then it could have been a flexible member, and this patch would have >> been a lot simpler. However since the data stored in this structure is >> ultimately exported to userspace, the order of this structure can't be changed. > Why not just remove the last element and allocate space for it after the > structure? Because that would still be employing VLAIS to solve the problem. The last element may be a zero-length array (a flexible member), not a VLA. Sadly both the last 2 elements in the struct need to be manually calculated, which is what we've done. > That would reduce the complexity of the patch and the unreadability > of the new code. No one is claiming this patch is more readable, merely that it is C99 compliant (though strictly speaking this patch is C89, C99 and C11 compliant). We tried to use macros to make it more readable in previous patches. The consensus was that macros were bad. > I realise that the alignment of type##_error is 'tricky' to determine. That is what makes it "unreadable". :( Behan -- Behan Webster behanw@converseincode.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-18 14:47 ` Behan Webster @ 2014-03-18 15:24 ` David Laight 0 siblings, 0 replies; 21+ messages in thread From: David Laight @ 2014-03-18 15:24 UTC (permalink / raw) To: 'Behan Webster', davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti From: Behan Webster > On 03/18/14 02:41, David Laight wrote: > > From: behanw@converseincode.com > >> From: Mark Charlebois <charlebm@gmail.com> > >> > >> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > >> xt_repldata.h with a C99 compliant flexible array member and then calculated > >> offsets to the other struct members. These other members aren't referenced by > >> name in this code, however this patch maintains the same memory layout and > >> padding as was previously accomplished using VLAIS. > >> > >> Had the original structure been ordered differently, with the entries VLA at > >> the end, then it could have been a flexible member, and this patch would have > >> been a lot simpler. However since the data stored in this structure is > >> ultimately exported to userspace, the order of this structure can't be changed. > > Why not just remove the last element and allocate space for it after the > > structure? > Because that would still be employing VLAIS to solve the problem. The > last element may be a zero-length array (a flexible member), not a VLA. > Sadly both the last 2 elements in the struct need to be manually > calculated, which is what we've done. So make the last element a 'flexible member' and then work out where the final field goes. Something like: struct p { struct a a; struct b b[]; } p = malloc(sizeof *p + n * sizeof (struct b) + alignof (struct c) + sizeof (struct c); struct c *c = (void *)&p->b[n] + (-offsetof(struct p, b[n]) & (alignof(struct c) - 1); David ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-18 15:24 ` David Laight 0 siblings, 0 replies; 21+ messages in thread From: David Laight @ 2014-03-18 15:24 UTC (permalink / raw) To: 'Behan Webster', davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1711 bytes --] From: Behan Webster > On 03/18/14 02:41, David Laight wrote: > > From: behanw@converseincode.com > >> From: Mark Charlebois <charlebm@gmail.com> > >> > >> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > >> xt_repldata.h with a C99 compliant flexible array member and then calculated > >> offsets to the other struct members. These other members aren't referenced by > >> name in this code, however this patch maintains the same memory layout and > >> padding as was previously accomplished using VLAIS. > >> > >> Had the original structure been ordered differently, with the entries VLA at > >> the end, then it could have been a flexible member, and this patch would have > >> been a lot simpler. However since the data stored in this structure is > >> ultimately exported to userspace, the order of this structure can't be changed. > > Why not just remove the last element and allocate space for it after the > > structure? > Because that would still be employing VLAIS to solve the problem. The > last element may be a zero-length array (a flexible member), not a VLA. > Sadly both the last 2 elements in the struct need to be manually > calculated, which is what we've done. So make the last element a 'flexible member' and then work out where the final field goes. Something like: struct p { struct a a; struct b b[]; } p = malloc(sizeof *p + n * sizeof (struct b) + alignof (struct c) + sizeof (struct c); struct c *c = (void *)&p->b[n] + (-offsetof(struct p, b[n]) & (alignof(struct c) - 1); David ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-18 15:24 ` David Laight (?) @ 2014-03-18 17:37 ` Behan Webster -1 siblings, 0 replies; 21+ messages in thread From: Behan Webster @ 2014-03-18 17:37 UTC (permalink / raw) To: David Laight, davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti On 03/18/14 08:24, David Laight wrote: > From: Behan Webster >> On 03/18/14 02:41, David Laight wrote: >>> From: behanw@converseincode.com >>>> From: Mark Charlebois <charlebm@gmail.com> >>>> >>>> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in >>>> xt_repldata.h with a C99 compliant flexible array member and then calculated >>>> offsets to the other struct members. These other members aren't referenced by >>>> name in this code, however this patch maintains the same memory layout and >>>> padding as was previously accomplished using VLAIS. >>>> >>>> Had the original structure been ordered differently, with the entries VLA at >>>> the end, then it could have been a flexible member, and this patch would have >>>> been a lot simpler. However since the data stored in this structure is >>>> ultimately exported to userspace, the order of this structure can't be changed. >>> Why not just remove the last element and allocate space for it after the >>> structure? >> Because that would still be employing VLAIS to solve the problem. The >> last element may be a zero-length array (a flexible member), not a VLA. >> Sadly both the last 2 elements in the struct need to be manually >> calculated, which is what we've done. > So make the last element a 'flexible member' and then work out where > the final field goes. > Something like: > struct p { > struct a a; > struct b b[]; > } p = malloc(sizeof *p + n * sizeof (struct b) + alignof (struct c) > + sizeof (struct c); > struct c *c = (void *)&p->b[n] + (-offsetof(struct p, b[n]) & (alignof(struct c) - 1); Oh, I see. Will fix. Thanks! Behan -- Behan Webster behanw@converseincode.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-18 15:24 ` David Laight @ 2014-03-19 1:50 ` behanw -1 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-19 1:50 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, torvalds, dwmw2, pageexec, David.Laight, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vin√≠cius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..343599e 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,23 +5,40 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + struct type##_standard entries[]; \ + } *tbl; \ + struct type##_error *term; \ + size_t entries_end = offsetof(typeof(*tbl), \ + entries[nhooks-1]) + sizeof(tbl->entries[0]); \ + size_t term_offset = (entries_end + __alignof__(*term) - 1) \ + & ~(__alignof__(*term) - 1); \ + size_t term_end = term_offset + sizeof(*term); \ + size_t tbl_sz = (term_end + __alignof__(tbl->repl) - 1) \ + & ~(__alignof__(tbl->repl) - 1); \ + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-19 1:50 ` behanw 0 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-19 1:50 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, torvalds, dwmw2, pageexec, David.Laight, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..343599e 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,23 +5,40 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + struct type##_standard entries[]; \ + } *tbl; \ + struct type##_error *term; \ + size_t entries_end = offsetof(typeof(*tbl), \ + entries[nhooks-1]) + sizeof(tbl->entries[0]); \ + size_t term_offset = (entries_end + __alignof__(*term) - 1) \ + & ~(__alignof__(*term) - 1); \ + size_t term_end = term_offset + sizeof(*term); \ + size_t tbl_sz = (term_end + __alignof__(tbl->repl) - 1) \ + & ~(__alignof__(tbl->repl) - 1); \ + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* RE: [PATCH v3] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-19 1:50 ` behanw (?) @ 2014-03-19 9:52 ` David Laight -1 siblings, 0 replies; 21+ messages in thread From: David Laight @ 2014-03-19 9:52 UTC (permalink / raw) To: 'behanw@converseincode.com', davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti From: behanw@converseincode.com > From: Mark Charlebois <charlebm@gmail.com> > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > xt_repldata.h with a C99 compliant flexible array member and then calculated > offsets to the other struct members. These other members aren't referenced by > name in this code, however this patch maintains the same memory layout and > padding as was previously accomplished using VLAIS. > > Had the original structure been ordered differently, with the entries VLA at > the end, then it could have been a flexible member, and this patch would have > been a lot simpler. However since the data stored in this structure is > ultimately exported to userspace, the order of this structure can't be changed. > > This patch makes no attempt to change the existing behavior, merely the way in > which the current layout is accomplished using standard C99 constructs. As such > the code can now be compiled with either gcc or clang. > > Author: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Behan Webster <behanw@converseincode.com> > Signed-off-by: Vin√≠cius Tinti <viniciustinti@gmail.com> > --- > net/netfilter/xt_repldata.h | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > index 6efe4e5..343599e 100644 > --- a/net/netfilter/xt_repldata.h > +++ b/net/netfilter/xt_repldata.h > @@ -5,23 +5,40 @@ > * they serve as the hanging-off data accessed through repl.data[]. > */ > > +/* tbl has the following structure equivalent, but is C99 compliant: > + * struct { > + * struct type##_replace repl; > + * struct type##_standard entries[nhooks]; > + * struct type##_error term; > + * } *tbl; > + */ > + > #define xt_alloc_initial_table(type, typ2) ({ \ > unsigned int hook_mask = info->valid_hooks; \ > unsigned int nhooks = hweight32(hook_mask); \ > unsigned int bytes = 0, hooknum = 0, i = 0; \ > struct { \ > struct type##_replace repl; \ > - struct type##_standard entries[nhooks]; \ > - struct type##_error term; \ > - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ > + struct type##_standard entries[]; \ > + } *tbl; \ > + struct type##_error *term; \ > + size_t entries_end = offsetof(typeof(*tbl), \ > + entries[nhooks-1]) + sizeof(tbl->entries[0]); \ Is the compiler complaining about: offsetof(typeof(*tbl), entries[nhooks]) If it does it is a PITA. > + size_t term_offset = (entries_end + __alignof__(*term) - 1) \ > + & ~(__alignof__(*term) - 1); \ You've not tested this - the () are in the wrong places. > + size_t term_end = term_offset + sizeof(*term); \ > + size_t tbl_sz = (term_end + __alignof__(tbl->repl) - 1) \ > + & ~(__alignof__(tbl->repl) - 1); \ > + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ The number of temporary variables make the above hard to read. I'm not at all sure you need to worry about the trailing alignment. It rather depends on how the final data is used. If the combined buffer is copied to userspace you may not be copying all of the required data. It might be easier to call copytouser() twice. > if (tbl == NULL) \ > return NULL; \ > + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ > + *term = (struct type##_error)typ2##_ERROR_INIT; \ David ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v3] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-19 9:52 ` David Laight 0 siblings, 0 replies; 21+ messages in thread From: David Laight @ 2014-03-19 9:52 UTC (permalink / raw) To: 'behanw@converseincode.com', davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3654 bytes --] From: behanw@converseincode.com > From: Mark Charlebois <charlebm@gmail.com> > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > xt_repldata.h with a C99 compliant flexible array member and then calculated > offsets to the other struct members. These other members aren't referenced by > name in this code, however this patch maintains the same memory layout and > padding as was previously accomplished using VLAIS. > > Had the original structure been ordered differently, with the entries VLA at > the end, then it could have been a flexible member, and this patch would have > been a lot simpler. However since the data stored in this structure is > ultimately exported to userspace, the order of this structure can't be changed. > > This patch makes no attempt to change the existing behavior, merely the way in > which the current layout is accomplished using standard C99 constructs. As such > the code can now be compiled with either gcc or clang. > > Author: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Behan Webster <behanw@converseincode.com> > Signed-off-by: VinÃcius Tinti <viniciustinti@gmail.com> > --- > net/netfilter/xt_repldata.h | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > index 6efe4e5..343599e 100644 > --- a/net/netfilter/xt_repldata.h > +++ b/net/netfilter/xt_repldata.h > @@ -5,23 +5,40 @@ > * they serve as the hanging-off data accessed through repl.data[]. > */ > > +/* tbl has the following structure equivalent, but is C99 compliant: > + * struct { > + * struct type##_replace repl; > + * struct type##_standard entries[nhooks]; > + * struct type##_error term; > + * } *tbl; > + */ > + > #define xt_alloc_initial_table(type, typ2) ({ \ > unsigned int hook_mask = info->valid_hooks; \ > unsigned int nhooks = hweight32(hook_mask); \ > unsigned int bytes = 0, hooknum = 0, i = 0; \ > struct { \ > struct type##_replace repl; \ > - struct type##_standard entries[nhooks]; \ > - struct type##_error term; \ > - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ > + struct type##_standard entries[]; \ > + } *tbl; \ > + struct type##_error *term; \ > + size_t entries_end = offsetof(typeof(*tbl), \ > + entries[nhooks-1]) + sizeof(tbl->entries[0]); \ Is the compiler complaining about: offsetof(typeof(*tbl), entries[nhooks]) If it does it is a PITA. > + size_t term_offset = (entries_end + __alignof__(*term) - 1) \ > + & ~(__alignof__(*term) - 1); \ You've not tested this - the () are in the wrong places. > + size_t term_end = term_offset + sizeof(*term); \ > + size_t tbl_sz = (term_end + __alignof__(tbl->repl) - 1) \ > + & ~(__alignof__(tbl->repl) - 1); \ > + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ The number of temporary variables make the above hard to read. I'm not at all sure you need to worry about the trailing alignment. It rather depends on how the final data is used. If the combined buffer is copied to userspace you may not be copying all of the required data. It might be easier to call copytouser() twice. > if (tbl == NULL) \ > return NULL; \ > + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ > + *term = (struct type##_error)typ2##_ERROR_INIT; \ David ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v3] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-19 9:52 ` David Laight 0 siblings, 0 replies; 21+ messages in thread From: David Laight @ 2014-03-19 9:52 UTC (permalink / raw) To: 'behanw@converseincode.com', davem@davemloft.net Cc: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, dwmw2@infradead.org, pageexec@freemail.hu, Mark Charlebois, Vinícius Tinti From: behanw@converseincode.com > From: Mark Charlebois <charlebm@gmail.com> > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > xt_repldata.h with a C99 compliant flexible array member and then calculated > offsets to the other struct members. These other members aren't referenced by > name in this code, however this patch maintains the same memory layout and > padding as was previously accomplished using VLAIS. > > Had the original structure been ordered differently, with the entries VLA at > the end, then it could have been a flexible member, and this patch would have > been a lot simpler. However since the data stored in this structure is > ultimately exported to userspace, the order of this structure can't be changed. > > This patch makes no attempt to change the existing behavior, merely the way in > which the current layout is accomplished using standard C99 constructs. As such > the code can now be compiled with either gcc or clang. > > Author: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Behan Webster <behanw@converseincode.com> > Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> > --- > net/netfilter/xt_repldata.h | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > index 6efe4e5..343599e 100644 > --- a/net/netfilter/xt_repldata.h > +++ b/net/netfilter/xt_repldata.h > @@ -5,23 +5,40 @@ > * they serve as the hanging-off data accessed through repl.data[]. > */ > > +/* tbl has the following structure equivalent, but is C99 compliant: > + * struct { > + * struct type##_replace repl; > + * struct type##_standard entries[nhooks]; > + * struct type##_error term; > + * } *tbl; > + */ > + > #define xt_alloc_initial_table(type, typ2) ({ \ > unsigned int hook_mask = info->valid_hooks; \ > unsigned int nhooks = hweight32(hook_mask); \ > unsigned int bytes = 0, hooknum = 0, i = 0; \ > struct { \ > struct type##_replace repl; \ > - struct type##_standard entries[nhooks]; \ > - struct type##_error term; \ > - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ > + struct type##_standard entries[]; \ > + } *tbl; \ > + struct type##_error *term; \ > + size_t entries_end = offsetof(typeof(*tbl), \ > + entries[nhooks-1]) + sizeof(tbl->entries[0]); \ Is the compiler complaining about: offsetof(typeof(*tbl), entries[nhooks]) If it does it is a PITA. > + size_t term_offset = (entries_end + __alignof__(*term) - 1) \ > + & ~(__alignof__(*term) - 1); \ You've not tested this - the () are in the wrong places. > + size_t term_end = term_offset + sizeof(*term); \ > + size_t tbl_sz = (term_end + __alignof__(tbl->repl) - 1) \ > + & ~(__alignof__(tbl->repl) - 1); \ > + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ The number of temporary variables make the above hard to read. I'm not at all sure you need to worry about the trailing alignment. It rather depends on how the final data is used. If the combined buffer is copied to userspace you may not be copying all of the required data. It might be easier to call copytouser() twice. > if (tbl == NULL) \ > return NULL; \ > + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ > + *term = (struct type##_error)typ2##_ERROR_INIT; \ David ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-19 9:52 ` David Laight @ 2014-03-19 17:25 ` Mark Charlebois -1 siblings, 0 replies; 21+ messages in thread From: Mark Charlebois @ 2014-03-19 17:25 UTC (permalink / raw) To: David Laight Cc: 'behanw@converseincode.com', davem@davemloft.net, pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, dwmw2@infradead.org, pageexec@freemail.hu, Vinícius Tinti On Wed, Mar 19, 2014 at 09:52:40AM +0000, David Laight wrote: > From: behanw@converseincode.com > > From: Mark Charlebois <charlebm@gmail.com> > > > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > > xt_repldata.h with a C99 compliant flexible array member and then calculated > > offsets to the other struct members. These other members aren't referenced by > > name in this code, however this patch maintains the same memory layout and > > padding as was previously accomplished using VLAIS. > > > > Had the original structure been ordered differently, with the entries VLA at > > the end, then it could have been a flexible member, and this patch would have > > been a lot simpler. However since the data stored in this structure is > > ultimately exported to userspace, the order of this structure can't be changed. > > > > This patch makes no attempt to change the existing behavior, merely the way in > > which the current layout is accomplished using standard C99 constructs. As such > > the code can now be compiled with either gcc or clang. > > > > Author: Mark Charlebois <charlebm@gmail.com> > > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > > Signed-off-by: Behan Webster <behanw@converseincode.com> > > Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> > > --- > > net/netfilter/xt_repldata.h | 27 ++++++++++++++++++++++----- > > 1 file changed, 22 insertions(+), 5 deletions(-) > > > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > > index 6efe4e5..343599e 100644 > > --- a/net/netfilter/xt_repldata.h > > +++ b/net/netfilter/xt_repldata.h > > @@ -5,23 +5,40 @@ > > * they serve as the hanging-off data accessed through repl.data[]. > > */ > > > > +/* tbl has the following structure equivalent, but is C99 compliant: > > + * struct { > > + * struct type##_replace repl; > > + * struct type##_standard entries[nhooks]; > > + * struct type##_error term; > > + * } *tbl; > > + */ > > + > > #define xt_alloc_initial_table(type, typ2) ({ \ > > unsigned int hook_mask = info->valid_hooks; \ > > unsigned int nhooks = hweight32(hook_mask); \ > > unsigned int bytes = 0, hooknum = 0, i = 0; \ > > struct { \ > > struct type##_replace repl; \ > > - struct type##_standard entries[nhooks]; \ > > - struct type##_error term; \ > > - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ > > + struct type##_standard entries[]; \ > > + } *tbl; \ > > + struct type##_error *term; \ > > + size_t entries_end = offsetof(typeof(*tbl), \ > > + entries[nhooks-1]) + sizeof(tbl->entries[0]); \ > > Is the compiler complaining about: > offsetof(typeof(*tbl), entries[nhooks]) > If it does it is a PITA. > > > + size_t term_offset = (entries_end + __alignof__(*term) - 1) \ > > + & ~(__alignof__(*term) - 1); \ > > You've not tested this - the () are in the wrong places. I have tested it with both clang and gcc. Unit test is at http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=test/unit/vlais/netfilter.c;h=7adc255e47e15a252d2bda7af7ae217ac683c25e;hb=HEAD Basic calulation of a new offset is: new offset = (offset + align - 1) & ~(align - 1) The parenthesis seem correct to me. > > > + size_t term_end = term_offset + sizeof(*term); \ > > + size_t tbl_sz = (term_end + __alignof__(tbl->repl) - 1) \ > > + & ~(__alignof__(tbl->repl) - 1); \ > > + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ > > The number of temporary variables make the above hard to read. > I'm not at all sure you need to worry about the trailing alignment. > It rather depends on how the final data is used. > If the combined buffer is copied to userspace you may not > be copying all of the required data. > It might be easier to call copytouser() twice. I can try to remove more variables if that is prefered. The existing memory layout was preserved exactly so that however the data currently used will be unaffected. I can remove the trailing alignment but was not 100% sure it wasn't needed. > > > if (tbl == NULL) \ > > return NULL; \ > > + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > > strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > > - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ > > + *term = (struct type##_error)typ2##_ERROR_INIT; \ > > David > Mark -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-19 17:25 ` Mark Charlebois 0 siblings, 0 replies; 21+ messages in thread From: Mark Charlebois @ 2014-03-19 17:25 UTC (permalink / raw) To: David Laight Cc: 'behanw@converseincode.com', davem@davemloft.net, pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, dwmw2@infradead.org, pageexec@freemail.hu, Vinícius Tinti On Wed, Mar 19, 2014 at 09:52:40AM +0000, David Laight wrote: > From: behanw@converseincode.com > > From: Mark Charlebois <charlebm@gmail.com> > > > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > > xt_repldata.h with a C99 compliant flexible array member and then calculated > > offsets to the other struct members. These other members aren't referenced by > > name in this code, however this patch maintains the same memory layout and > > padding as was previously accomplished using VLAIS. > > > > Had the original structure been ordered differently, with the entries VLA at > > the end, then it could have been a flexible member, and this patch would have > > been a lot simpler. However since the data stored in this structure is > > ultimately exported to userspace, the order of this structure can't be changed. > > > > This patch makes no attempt to change the existing behavior, merely the way in > > which the current layout is accomplished using standard C99 constructs. As such > > the code can now be compiled with either gcc or clang. > > > > Author: Mark Charlebois <charlebm@gmail.com> > > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > > Signed-off-by: Behan Webster <behanw@converseincode.com> > > Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> > > --- > > net/netfilter/xt_repldata.h | 27 ++++++++++++++++++++++----- > > 1 file changed, 22 insertions(+), 5 deletions(-) > > > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > > index 6efe4e5..343599e 100644 > > --- a/net/netfilter/xt_repldata.h > > +++ b/net/netfilter/xt_repldata.h > > @@ -5,23 +5,40 @@ > > * they serve as the hanging-off data accessed through repl.data[]. > > */ > > > > +/* tbl has the following structure equivalent, but is C99 compliant: > > + * struct { > > + * struct type##_replace repl; > > + * struct type##_standard entries[nhooks]; > > + * struct type##_error term; > > + * } *tbl; > > + */ > > + > > #define xt_alloc_initial_table(type, typ2) ({ \ > > unsigned int hook_mask = info->valid_hooks; \ > > unsigned int nhooks = hweight32(hook_mask); \ > > unsigned int bytes = 0, hooknum = 0, i = 0; \ > > struct { \ > > struct type##_replace repl; \ > > - struct type##_standard entries[nhooks]; \ > > - struct type##_error term; \ > > - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ > > + struct type##_standard entries[]; \ > > + } *tbl; \ > > + struct type##_error *term; \ > > + size_t entries_end = offsetof(typeof(*tbl), \ > > + entries[nhooks-1]) + sizeof(tbl->entries[0]); \ > > Is the compiler complaining about: > offsetof(typeof(*tbl), entries[nhooks]) > If it does it is a PITA. > > > + size_t term_offset = (entries_end + __alignof__(*term) - 1) \ > > + & ~(__alignof__(*term) - 1); \ > > You've not tested this - the () are in the wrong places. I have tested it with both clang and gcc. Unit test is at http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=test/unit/vlais/netfilter.c;h=7adc255e47e15a252d2bda7af7ae217ac683c25e;hb=HEAD Basic calulation of a new offset is: new offset = (offset + align - 1) & ~(align - 1) The parenthesis seem correct to me. > > > + size_t term_end = term_offset + sizeof(*term); \ > > + size_t tbl_sz = (term_end + __alignof__(tbl->repl) - 1) \ > > + & ~(__alignof__(tbl->repl) - 1); \ > > + tbl = kzalloc(tbl_sz, GFP_KERNEL); \ > > The number of temporary variables make the above hard to read. > I'm not at all sure you need to worry about the trailing alignment. > It rather depends on how the final data is used. > If the combined buffer is copied to userspace you may not > be copying all of the required data. > It might be easier to call copytouser() twice. I can try to remove more variables if that is prefered. The existing memory layout was preserved exactly so that however the data currently used will be unaffected. I can remove the trailing alignment but was not 100% sure it wasn't needed. > > > if (tbl == NULL) \ > > return NULL; \ > > + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > > strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > > - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ > > + *term = (struct type##_error)typ2##_ERROR_INIT; \ > > David > Mark ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-19 9:52 ` David Laight (?) @ 2014-03-23 6:55 ` behanw -1 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-23 6:55 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. This version of the patch removes the trailing alignment that the VLAIS structure would allocate in order to simplify the patch. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vin√≠cius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..8fd3241 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,23 +5,35 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + struct type##_standard entries[]; \ + } *tbl; \ + struct type##_error *term; \ + size_t term_offset = (offsetof(typeof(*tbl), entries[nhooks]) + \ + __alignof__(*term) - 1) & ~(__alignof__(*term) - 1); \ + tbl = kzalloc(term_offset + sizeof(*term), GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-23 6:55 ` behanw 0 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-23 6:55 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. This version of the patch removes the trailing alignment that the VLAIS structure would allocate in order to simplify the patch. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..8fd3241 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,23 +5,35 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + struct type##_standard entries[]; \ + } *tbl; \ + struct type##_error *term; \ + size_t term_offset = (offsetof(typeof(*tbl), entries[nhooks]) + \ + __alignof__(*term) - 1) & ~(__alignof__(*term) - 1); \ + tbl = kzalloc(term_offset + sizeof(*term), GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-03-23 6:55 ` behanw 0 siblings, 0 replies; 21+ messages in thread From: behanw @ 2014-03-23 6:55 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Behan Webster, Vinícius Tinti From: Mark Charlebois <charlebm@gmail.com> Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in xt_repldata.h with a C99 compliant flexible array member and then calculated offsets to the other struct members. These other members aren't referenced by name in this code, however this patch maintains the same memory layout and padding as was previously accomplished using VLAIS. Had the original structure been ordered differently, with the entries VLA at the end, then it could have been a flexible member, and this patch would have been a lot simpler. However since the data stored in this structure is ultimately exported to userspace, the order of this structure can't be changed. This patch makes no attempt to change the existing behavior, merely the way in which the current layout is accomplished using standard C99 constructs. As such the code can now be compiled with either gcc or clang. This version of the patch removes the trailing alignment that the VLAIS structure would allocate in order to simplify the patch. Author: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Mark Charlebois <charlebm@gmail.com> Signed-off-by: Behan Webster <behanw@converseincode.com> Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> --- net/netfilter/xt_repldata.h | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h index 6efe4e5..8fd3241 100644 --- a/net/netfilter/xt_repldata.h +++ b/net/netfilter/xt_repldata.h @@ -5,23 +5,35 @@ * they serve as the hanging-off data accessed through repl.data[]. */ +/* tbl has the following structure equivalent, but is C99 compliant: + * struct { + * struct type##_replace repl; + * struct type##_standard entries[nhooks]; + * struct type##_error term; + * } *tbl; + */ + #define xt_alloc_initial_table(type, typ2) ({ \ unsigned int hook_mask = info->valid_hooks; \ unsigned int nhooks = hweight32(hook_mask); \ unsigned int bytes = 0, hooknum = 0, i = 0; \ struct { \ struct type##_replace repl; \ - struct type##_standard entries[nhooks]; \ - struct type##_error term; \ - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ + struct type##_standard entries[]; \ + } *tbl; \ + struct type##_error *term; \ + size_t term_offset = (offsetof(typeof(*tbl), entries[nhooks]) + \ + __alignof__(*term) - 1) & ~(__alignof__(*term) - 1); \ + tbl = kzalloc(term_offset + sizeof(*term), GFP_KERNEL); \ if (tbl == NULL) \ return NULL; \ + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ + *term = (struct type##_error)typ2##_ERROR_INIT; \ tbl->repl.valid_hooks = hook_mask; \ tbl->repl.num_entries = nhooks + 1; \ tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ - sizeof(struct type##_error); \ + sizeof(struct type##_error); \ for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ if (!(hook_mask & 1)) \ continue; \ -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4] net: netfilter: LLVMLinux: vlais-netfilter 2014-03-23 6:55 ` behanw @ 2014-04-18 0:40 ` Behan Webster -1 siblings, 0 replies; 21+ messages in thread From: Behan Webster @ 2014-04-18 0:40 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Vinícius Tinti On 03/22/14 23:55, behanw@converseincode.com wrote: > From: Mark Charlebois <charlebm@gmail.com> > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > xt_repldata.h with a C99 compliant flexible array member and then calculated > offsets to the other struct members. These other members aren't referenced by > name in this code, however this patch maintains the same memory layout and > padding as was previously accomplished using VLAIS. > > Had the original structure been ordered differently, with the entries VLA at > the end, then it could have been a flexible member, and this patch would have > been a lot simpler. However since the data stored in this structure is > ultimately exported to userspace, the order of this structure can't be changed. > > This patch makes no attempt to change the existing behavior, merely the way in > which the current layout is accomplished using standard C99 constructs. As such > the code can now be compiled with either gcc or clang. > > This version of the patch removes the trailing alignment that the VLAIS > structure would allocate in order to simplify the patch. > > Author: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Behan Webster <behanw@converseincode.com> > Signed-off-by: Vin√≠cius Tinti <viniciustinti@gmail.com> > --- > net/netfilter/xt_repldata.h | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > index 6efe4e5..8fd3241 100644 > --- a/net/netfilter/xt_repldata.h > +++ b/net/netfilter/xt_repldata.h > @@ -5,23 +5,35 @@ > * they serve as the hanging-off data accessed through repl.data[]. > */ > > +/* tbl has the following structure equivalent, but is C99 compliant: > + * struct { > + * struct type##_replace repl; > + * struct type##_standard entries[nhooks]; > + * struct type##_error term; > + * } *tbl; > + */ > + > #define xt_alloc_initial_table(type, typ2) ({ \ > unsigned int hook_mask = info->valid_hooks; \ > unsigned int nhooks = hweight32(hook_mask); \ > unsigned int bytes = 0, hooknum = 0, i = 0; \ > struct { \ > struct type##_replace repl; \ > - struct type##_standard entries[nhooks]; \ > - struct type##_error term; \ > - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ > + struct type##_standard entries[]; \ > + } *tbl; \ > + struct type##_error *term; \ > + size_t term_offset = (offsetof(typeof(*tbl), entries[nhooks]) + \ > + __alignof__(*term) - 1) & ~(__alignof__(*term) - 1); \ > + tbl = kzalloc(term_offset + sizeof(*term), GFP_KERNEL); \ > if (tbl == NULL) \ > return NULL; \ > + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ > + *term = (struct type##_error)typ2##_ERROR_INIT; \ > tbl->repl.valid_hooks = hook_mask; \ > tbl->repl.num_entries = nhooks + 1; \ > tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ > - sizeof(struct type##_error); \ > + sizeof(struct type##_error); \ > for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ > if (!(hook_mask & 1)) \ > continue; \ Any further feedback about this patch? Behan -- Behan Webster behanw@converseincode.com ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4] net: netfilter: LLVMLinux: vlais-netfilter @ 2014-04-18 0:40 ` Behan Webster 0 siblings, 0 replies; 21+ messages in thread From: Behan Webster @ 2014-04-18 0:40 UTC (permalink / raw) To: davem Cc: pablo, kaber, kadlec, netfilter-devel, netfilter, coreteam, netdev, linux-kernel, dwmw2, pageexec, Mark Charlebois, Vinícius Tinti On 03/22/14 23:55, behanw@converseincode.com wrote: > From: Mark Charlebois <charlebm@gmail.com> > > Replaced non-standard C use of Variable Length Arrays In Structs (VLAIS) in > xt_repldata.h with a C99 compliant flexible array member and then calculated > offsets to the other struct members. These other members aren't referenced by > name in this code, however this patch maintains the same memory layout and > padding as was previously accomplished using VLAIS. > > Had the original structure been ordered differently, with the entries VLA at > the end, then it could have been a flexible member, and this patch would have > been a lot simpler. However since the data stored in this structure is > ultimately exported to userspace, the order of this structure can't be changed. > > This patch makes no attempt to change the existing behavior, merely the way in > which the current layout is accomplished using standard C99 constructs. As such > the code can now be compiled with either gcc or clang. > > This version of the patch removes the trailing alignment that the VLAIS > structure would allocate in order to simplify the patch. > > Author: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Mark Charlebois <charlebm@gmail.com> > Signed-off-by: Behan Webster <behanw@converseincode.com> > Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com> > --- > net/netfilter/xt_repldata.h | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > index 6efe4e5..8fd3241 100644 > --- a/net/netfilter/xt_repldata.h > +++ b/net/netfilter/xt_repldata.h > @@ -5,23 +5,35 @@ > * they serve as the hanging-off data accessed through repl.data[]. > */ > > +/* tbl has the following structure equivalent, but is C99 compliant: > + * struct { > + * struct type##_replace repl; > + * struct type##_standard entries[nhooks]; > + * struct type##_error term; > + * } *tbl; > + */ > + > #define xt_alloc_initial_table(type, typ2) ({ \ > unsigned int hook_mask = info->valid_hooks; \ > unsigned int nhooks = hweight32(hook_mask); \ > unsigned int bytes = 0, hooknum = 0, i = 0; \ > struct { \ > struct type##_replace repl; \ > - struct type##_standard entries[nhooks]; \ > - struct type##_error term; \ > - } *tbl = kzalloc(sizeof(*tbl), GFP_KERNEL); \ > + struct type##_standard entries[]; \ > + } *tbl; \ > + struct type##_error *term; \ > + size_t term_offset = (offsetof(typeof(*tbl), entries[nhooks]) + \ > + __alignof__(*term) - 1) & ~(__alignof__(*term) - 1); \ > + tbl = kzalloc(term_offset + sizeof(*term), GFP_KERNEL); \ > if (tbl == NULL) \ > return NULL; \ > + term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > - tbl->term = (struct type##_error)typ2##_ERROR_INIT; \ > + *term = (struct type##_error)typ2##_ERROR_INIT; \ > tbl->repl.valid_hooks = hook_mask; \ > tbl->repl.num_entries = nhooks + 1; \ > tbl->repl.size = nhooks * sizeof(struct type##_standard) + \ > - sizeof(struct type##_error); \ > + sizeof(struct type##_error); \ > for (; hook_mask != 0; hook_mask >>= 1, ++hooknum) { \ > if (!(hook_mask & 1)) \ > continue; \ Any further feedback about this patch? Behan -- Behan Webster behanw@converseincode.com ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2014-04-18 0:40 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <022D9517-4804-47BB-B789-CCF25F25C1ED@me.com>
2014-03-18 6:12 ` [PATCH v2] net: netfilter: LLVMLinux: vlais-netfilter behanw
2014-03-18 6:12 ` behanw
2014-03-18 6:12 ` behanw
2014-03-18 9:41 ` David Laight
2014-03-18 9:41 ` David Laight
2014-03-18 14:47 ` Behan Webster
2014-03-18 15:24 ` David Laight
2014-03-18 15:24 ` David Laight
2014-03-18 17:37 ` Behan Webster
2014-03-19 1:50 ` [PATCH v3] " behanw
2014-03-19 1:50 ` behanw
2014-03-19 9:52 ` David Laight
2014-03-19 9:52 ` David Laight
2014-03-19 9:52 ` David Laight
2014-03-19 17:25 ` Mark Charlebois
2014-03-19 17:25 ` Mark Charlebois
2014-03-23 6:55 ` [PATCH v4] " behanw
2014-03-23 6:55 ` behanw
2014-03-23 6:55 ` behanw
2014-04-18 0:40 ` Behan Webster
2014-04-18 0:40 ` Behan Webster
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.