Re: [PATCH v2 1/3] Staging: Android: Fixes TODO file
On Wednesday 01 July 2015 07:57 PM, Dan Carpenter wrote: > On Wed, Jul 01, 2015 at 07:14:38PM +0530, Sohny Thomas wrote: >> >> - removed non-existent issue from TODO file >> kuid_t or uid_t not present in staging/android >> >> Signed-of-by: Sohny Thomas > > > The patch is all mangled. It barely seems to match the description but > I can't tell for sure because of the mangling. I sent an another version, Hope its not mangled now Thanks for your time. -Sohny > > regards, > dan carpenter > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 ] Staging: Android: Fixes for TODO file
Removed non-existent issue from TODO file
No instance of kuid_t or uid_t as mentioned by the file is present in
staging/android directory
Signed-off-by: Sohny Thomas
---
drivers/staging/android/TODO | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 06954cd..b15fb0d 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -5,13 +5,6 @@ TODO:
- make sure things build as modules properly
- add proper arch dependencies as needed
- audit userspace interfaces to make sure they are sane
- - kuid_t should never be exposed to user space as it is
- kernel internal type. Data structure for this kuid_t is:
- typedef struct {
- uid_t val;
- } kuid_t;
- - This bug is introduced by Xiong Zhou in the patch bd471258f2e09
- - ("staging: android: logger: use kuid_t instead of uid_t")
Please send patches to Greg Kroah-Hartman and Cc:
Brian Swetland
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] Staging: Android: Fixes TODO file
On Wednesday 01 July 2015 07:43 PM, Frans Klaver wrote: > On Wed, Jul 1, 2015 at 3:44 PM, Sohny Thomas wrote: >> >> - removed non-existent issue from TODO file >> kuid_t or uid_t not present in staging/android >> >> Signed-of-by: Sohny Thomas > > s,-of-,-off-, > > You can remove the leading dash (-). Could you elaborate on why this > issue is non existent? Thanks for catching the off. According to the TODO the kuid_t is not present in any files under staging/android So I said its non-existent > > Thanks, > Frans > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] Staging: android: uapi: Fix coding style typedef issue
As per checkpatch.pl remove unnecessary typedef
Here ion_user_handle_t was used only in the header .
Signed-of-by: Sohny Thomas
---
drivers/staging/android/uapi/ion.h | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 68a14b4..9897403 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -20,7 +20,6 @@
#include
#include
-typedef int ion_user_handle_t;
/**
* enum ion_heap_types - list of all possible types of heaps
@@ -88,7 +87,7 @@ struct ion_allocation_data {
size_t align;
unsigned int heap_id_mask;
unsigned int flags;
- ion_user_handle_t handle;
+ int handle;
};
/**
@@ -102,7 +101,7 @@ struct ion_allocation_data {
* provides the file descriptor and the kernel returns the handle.
*/
struct ion_fd_data {
- ion_user_handle_t handle;
+ int handle;
int fd;
};
@@ -111,7 +110,7 @@ struct ion_fd_data {
* @handle:a handle
*/
struct ion_handle_data {
- ion_user_handle_t handle;
+ int handle;
};
/**
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] Staging: android: ion: fix coding style
- Fixed 80 char limit exceeding line
and a newline after declarations as per checkpatch.pl
Signed-of-by: Sohny Thomas
---
drivers/staging/android/ion/ion.c| 1 +
drivers/staging/android/ion/ion_chunk_heap.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 6f48112..e44f5e6 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1106,6 +1106,7 @@ struct dma_buf *ion_share_dma_buf(struct ion_client
*client,
struct ion_buffer *buffer;
struct dma_buf *dmabuf;
bool valid_handle;
+
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
mutex_lock(>lock);
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 5474615..0813163 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -173,8 +173,8 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
chunk_heap->heap.ops = _heap_ops;
chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
- pr_debug("%s: base %lu size %zu align %ld\n", __func__,
chunk_heap->base,
- heap_data->size, heap_data->align);
+ pr_debug("%s: base %lu size %zu align %ld\n", __func__,
+ chunk_heap->base, heap_data->size, heap_data->align);
return _heap->heap;
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] Staging: Android: Fixes TODO file
- removed non-existent issue from TODO file
kuid_t or uid_t not present in staging/android
Signed-of-by: Sohny Thomas
---
drivers/staging/android/TODO | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 06954cd..b15fb0d 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -5,13 +5,6 @@ TODO:
- make sure things build as modules properly
- add proper arch dependencies as needed
- audit userspace interfaces to make sure they are sane
- - kuid_t should never be exposed to user space as it is
- kernel internal type. Data structure for this kuid_t is:
- typedef struct {
- uid_t val;
- } kuid_t;
- - This bug is introduced by Xiong Zhou in the patch bd471258f2e09
- - ("staging: android: logger: use kuid_t instead of uid_t")
Please send patches to Greg Kroah-Hartman and Cc:
Brian Swetland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/3] Staging: android: Fix coding style and TODO file
These set of patches Fixes the following issues in the android staging driver - removed non-existent issue from TODO file kuid_t or uid_t not present in staging/android - fixed 80 char limit exceeding line - a newline after declarations as per checkpatch.pl - fixed an unnecessary typedef as reported by checkpatch.pl -Regards, Sohny -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: android: fix coding style and TODO file
On Wednesday 01 July 2015 05:59 PM, Frans Klaver wrote: > On Wed, Jul 1, 2015 at 2:22 PM, Sohny Thomas wrote: >> >> >> On Wednesday 01 July 2015 05:37 PM, Frans Klaver wrote: >>> On Wed, Jul 1, 2015 at 1:56 PM, Sohny Thomas wrote: >>>> - removed non-existant issue from TODO file >>> >>> s,existant,existent, >> Thanks missed that >>> >>>> kuid_t or uid_t not present in staging/android >>>> - fixed 80 char limit exceeding line >>>> - a newline after decelartions as per checkpatch.pl >>>> - fixed an unnecessary typedef as reported by checkpatch.pl >>> >>> Fix one issue per patch, please. >> Since these were all simple Fixes of about 1/2 lines , I thought to make a >> single patch. > > They are simple fixes, but a reviewer still has to figure out what > comment belongs to which code change. Since there's no reason for > these changes to be atomic, you might as well split them up to make > reviewing easier. > Yeap, V2 coming with changes and fixes Sudip's catch too. Thanks for your time. -Sohny > Frans > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: android: fix coding style and TODO file
On Wednesday 01 July 2015 05:37 PM, Frans Klaver wrote: > On Wed, Jul 1, 2015 at 1:56 PM, Sohny Thomas wrote: >> - removed non-existant issue from TODO file > > s,existant,existent, Thanks missed that > >> kuid_t or uid_t not present in staging/android >> - fixed 80 char limit exceeding line >> - a newline after decelartions as per checkpatch.pl >> - fixed an unnecessary typedef as reported by checkpatch.pl > > Fix one issue per patch, please. Since these were all simple Fixes of about 1/2 lines , I thought to make a single patch. > > Thanks, > Frans > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: android: fix coding style and TODO file
- removed non-existant issue from TODO file
kuid_t or uid_t not present in staging/android
- fixed 80 char limit exceeding line
- a newline after decelartions as per checkpatch.pl
- fixed an unnecessary typedef as reported by checkpatch.pl
---
drivers/staging/android/TODO | 7 ---
drivers/staging/android/ion/ion.c| 1 +
drivers/staging/android/ion/ion_chunk_heap.c | 4 ++--
drivers/staging/android/uapi/ion.h | 7 +++
4 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 06954cd..b15fb0d 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -5,13 +5,6 @@ TODO:
- make sure things build as modules properly
- add proper arch dependencies as needed
- audit userspace interfaces to make sure they are sane
- - kuid_t should never be exposed to user space as it is
- kernel internal type. Data structure for this kuid_t is:
- typedef struct {
- uid_t val;
- } kuid_t;
- - This bug is introduced by Xiong Zhou in the patch bd471258f2e09
- - ("staging: android: logger: use kuid_t instead of uid_t")
Please send patches to Greg Kroah-Hartman and Cc:
Brian Swetland
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 6f48112..e44f5e6 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1106,6 +1106,7 @@ struct dma_buf *ion_share_dma_buf(struct ion_client
*client,
struct ion_buffer *buffer;
struct dma_buf *dmabuf;
bool valid_handle;
+
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
mutex_lock(>lock);
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 5474615..0813163 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -173,8 +173,8 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
chunk_heap->heap.ops = _heap_ops;
chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
- pr_debug("%s: base %lu size %zu align %ld\n", __func__,
chunk_heap->base,
- heap_data->size, heap_data->align);
+ pr_debug("%s: base %lu size %zu align %ld\n", __func__,
+ chunk_heap->base, heap_data->size, heap_data->align);
return _heap->heap;
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 68a14b4..9897403 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -20,7 +20,6 @@
#include
#include
-typedef int ion_user_handle_t;
/**
* enum ion_heap_types - list of all possible types of heaps
@@ -88,7 +87,7 @@ struct ion_allocation_data {
size_t align;
unsigned int heap_id_mask;
unsigned int flags;
- ion_user_handle_t handle;
+ int handle;
};
/**
@@ -102,7 +101,7 @@ struct ion_allocation_data {
* provides the file descriptor and the kernel returns the handle.
*/
struct ion_fd_data {
- ion_user_handle_t handle;
+ int handle;
int fd;
};
@@ -111,7 +110,7 @@ struct ion_fd_data {
* @handle:a handle
*/
struct ion_handle_data {
- ion_user_handle_t handle;
+ int handle;
};
/**
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
, NULL);
- if (i) {
+ if (i)
return 1;
- }
- return 0;
+ else
+ return 0;
>>> No, now this will introduce a new checkpatch warning that "else is not
>>> required after return". why did you introduce this "else"?
>> I did this so that the code is more readable and understandable, I checked
>> and
>> checkpatch didn't call this out , so its clean.
>>
>> Otherwise the above code looks like this
>>
>> if(i)
>>return 1;
>> return 0;
>
> That looks fine.
>
> I haven't looked at the code in detail. Is it normal that the return
> values seem to be 0 1 and -1? Which values represent success and which
> represent an error? It is nicer to have the errors under if and success
> as a direct return at the end.
Here in this driver directory, return 1 means SUCCESS and return 0 means FAILURE
So you mean my code change is fine?
>
> julia
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
On Wednesday 01 July 2015 01:32 PM, Sudip Mukherjee wrote: > On Wed, Jul 01, 2015 at 01:06:48PM +0530, Sohny Thomas wrote: > >>> No, now this will introduce a new checkpatch warning that "else is not >>> required after return". why did you introduce this "else"? >> I did this so that the code is more readable and understandable, I >> checked and checkpatch didn't call this out , so its clean. >> >> Otherwise the above code looks like this >> >> if(i) >>return 1; >> return 0; > you should update your tree. virtpci folder has been deleted from > unisys driver. > As you are using an old tree, maybe that explains why checkpatch is not > giving the error. This is from linux-stable branch and I updated it just yesterday, so looks like the folders still there > > regards > sudip > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
Thanks for review, my answers inline
On 01-07-2015 12:27, Sudip Mukherjee wrote:
On Wed, Jul 01, 2015 at 03:05:45AM +0530, Sohny Thomas wrote:
FIX 2 unnecessary braces found by checkpatch.pl
Signed-off-by: Sohny Thomas
---
drivers/staging/unisys/virtpci/virtpci.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/unisys/virtpci/virtpci.c
b/drivers/staging/unisys/virtpci/virtpci.c
index d5ad017..f3674de 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -190,9 +190,10 @@ static int write_vbus_chp_info(struct
spar_vbus_channel_protocol *chan,
return -1;
off = sizeof(struct channel_header) + chan->hdr_info.chp_info_offset;
- if (chan->hdr_info.chp_info_offset == 0) {
+
+ if (chan->hdr_info.chp_info_offset == 0)
return -1;
- }
+
why you are inserting new line here?
I did it so that its readable, will remove it if not required
memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
return 0;
}
@@ -484,10 +485,10 @@ static int delete_vhba(struct del_virt_guestpart
*delparams)
i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
, NULL);
- if (i) {
+ if (i)
return 1;
- }
- return 0;
+ else
+ return 0;
No, now this will introduce a new checkpatch warning that "else is not
required after return". why did you introduce this "else"?
I did this so that the code is more readable and understandable, I
checked and checkpatch didn't call this out , so its clean.
Otherwise the above code looks like this
if(i)
return 1;
return 0;
regards
sudip
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
On Wednesday 01 July 2015 01:32 PM, Sudip Mukherjee wrote: On Wed, Jul 01, 2015 at 01:06:48PM +0530, Sohny Thomas wrote: snip No, now this will introduce a new checkpatch warning that else is not required after return. why did you introduce this else? I did this so that the code is more readable and understandable, I checked and checkpatch didn't call this out , so its clean. Otherwise the above code looks like this if(i) return 1; return 0; you should update your tree. virtpci folder has been deleted from unisys driver. As you are using an old tree, maybe that explains why checkpatch is not giving the error. This is from linux-stable branch and I updated it just yesterday, so looks like the folders still there regards sudip -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] Staging: android: uapi: Fix coding style typedef issue
As per checkpatch.pl remove unnecessary typedef
Here ion_user_handle_t was used only in the header .
Signed-of-by: Sohny Thomas sohnytho...@zoho.com
---
drivers/staging/android/uapi/ion.h | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 68a14b4..9897403 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -20,7 +20,6 @@
#include linux/ioctl.h
#include linux/types.h
-typedef int ion_user_handle_t;
/**
* enum ion_heap_types - list of all possible types of heaps
@@ -88,7 +87,7 @@ struct ion_allocation_data {
size_t align;
unsigned int heap_id_mask;
unsigned int flags;
- ion_user_handle_t handle;
+ int handle;
};
/**
@@ -102,7 +101,7 @@ struct ion_allocation_data {
* provides the file descriptor and the kernel returns the handle.
*/
struct ion_fd_data {
- ion_user_handle_t handle;
+ int handle;
int fd;
};
@@ -111,7 +110,7 @@ struct ion_fd_data {
* @handle:a handle
*/
struct ion_handle_data {
- ion_user_handle_t handle;
+ int handle;
};
/**
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/3] Staging: android: ion: fix coding style
- Fixed 80 char limit exceeding line and a newline after declarations as per checkpatch.pl Signed-of-by: Sohny Thomas sohnytho...@zoho.com --- drivers/staging/android/ion/ion.c| 1 + drivers/staging/android/ion/ion_chunk_heap.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 6f48112..e44f5e6 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -1106,6 +1106,7 @@ struct dma_buf *ion_share_dma_buf(struct ion_client *client, struct ion_buffer *buffer; struct dma_buf *dmabuf; bool valid_handle; + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); mutex_lock(client-lock); diff --git a/drivers/staging/android/ion/ion_chunk_heap.c b/drivers/staging/android/ion/ion_chunk_heap.c index 5474615..0813163 100644 --- a/drivers/staging/android/ion/ion_chunk_heap.c +++ b/drivers/staging/android/ion/ion_chunk_heap.c @@ -173,8 +173,8 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data) chunk_heap-heap.ops = chunk_heap_ops; chunk_heap-heap.type = ION_HEAP_TYPE_CHUNK; chunk_heap-heap.flags = ION_HEAP_FLAG_DEFER_FREE; - pr_debug(%s: base %lu size %zu align %ld\n, __func__, chunk_heap-base, - heap_data-size, heap_data-align); + pr_debug(%s: base %lu size %zu align %ld\n, __func__, + chunk_heap-base, heap_data-size, heap_data-align); return chunk_heap-heap; -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/3] Staging: android: Fix coding style and TODO file
These set of patches Fixes the following issues in the android staging driver - removed non-existent issue from TODO file kuid_t or uid_t not present in staging/android - fixed 80 char limit exceeding line - a newline after declarations as per checkpatch.pl - fixed an unnecessary typedef as reported by checkpatch.pl -Regards, Sohny -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] Staging: Android: Fixes TODO file
- removed non-existent issue from TODO file
kuid_t or uid_t not present in staging/android
Signed-of-by: Sohny Thomas sohnytho...@zoho.com
---
drivers/staging/android/TODO | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 06954cd..b15fb0d 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -5,13 +5,6 @@ TODO:
- make sure things build as modules properly
- add proper arch dependencies as needed
- audit userspace interfaces to make sure they are sane
- - kuid_t should never be exposed to user space as it is
- kernel internal type. Data structure for this kuid_t is:
- typedef struct {
- uid_t val;
- } kuid_t;
- - This bug is introduced by Xiong Zhou in the patch bd471258f2e09
- - (staging: android: logger: use kuid_t instead of uid_t)
Please send patches to Greg Kroah-Hartman g...@kroah.com and Cc:
Brian Swetland swetl...@google.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: android: fix coding style and TODO file
- removed non-existant issue from TODO file
kuid_t or uid_t not present in staging/android
- fixed 80 char limit exceeding line
- a newline after decelartions as per checkpatch.pl
- fixed an unnecessary typedef as reported by checkpatch.pl
---
drivers/staging/android/TODO | 7 ---
drivers/staging/android/ion/ion.c| 1 +
drivers/staging/android/ion/ion_chunk_heap.c | 4 ++--
drivers/staging/android/uapi/ion.h | 7 +++
4 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 06954cd..b15fb0d 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -5,13 +5,6 @@ TODO:
- make sure things build as modules properly
- add proper arch dependencies as needed
- audit userspace interfaces to make sure they are sane
- - kuid_t should never be exposed to user space as it is
- kernel internal type. Data structure for this kuid_t is:
- typedef struct {
- uid_t val;
- } kuid_t;
- - This bug is introduced by Xiong Zhou in the patch bd471258f2e09
- - (staging: android: logger: use kuid_t instead of uid_t)
Please send patches to Greg Kroah-Hartman g...@kroah.com and Cc:
Brian Swetland swetl...@google.com
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 6f48112..e44f5e6 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1106,6 +1106,7 @@ struct dma_buf *ion_share_dma_buf(struct ion_client
*client,
struct ion_buffer *buffer;
struct dma_buf *dmabuf;
bool valid_handle;
+
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
mutex_lock(client-lock);
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 5474615..0813163 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -173,8 +173,8 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
chunk_heap-heap.ops = chunk_heap_ops;
chunk_heap-heap.type = ION_HEAP_TYPE_CHUNK;
chunk_heap-heap.flags = ION_HEAP_FLAG_DEFER_FREE;
- pr_debug(%s: base %lu size %zu align %ld\n, __func__,
chunk_heap-base,
- heap_data-size, heap_data-align);
+ pr_debug(%s: base %lu size %zu align %ld\n, __func__,
+ chunk_heap-base, heap_data-size, heap_data-align);
return chunk_heap-heap;
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 68a14b4..9897403 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -20,7 +20,6 @@
#include linux/ioctl.h
#include linux/types.h
-typedef int ion_user_handle_t;
/**
* enum ion_heap_types - list of all possible types of heaps
@@ -88,7 +87,7 @@ struct ion_allocation_data {
size_t align;
unsigned int heap_id_mask;
unsigned int flags;
- ion_user_handle_t handle;
+ int handle;
};
/**
@@ -102,7 +101,7 @@ struct ion_allocation_data {
* provides the file descriptor and the kernel returns the handle.
*/
struct ion_fd_data {
- ion_user_handle_t handle;
+ int handle;
int fd;
};
@@ -111,7 +110,7 @@ struct ion_fd_data {
* @handle:a handle
*/
struct ion_handle_data {
- ion_user_handle_t handle;
+ int handle;
};
/**
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
scsi.wwnn, NULL);
- if (i) {
+ if (i)
return 1;
- }
- return 0;
+ else
+ return 0;
No, now this will introduce a new checkpatch warning that else is not
required after return. why did you introduce this else?
I did this so that the code is more readable and understandable, I checked
and
checkpatch didn't call this out , so its clean.
Otherwise the above code looks like this
if(i)
return 1;
return 0;
That looks fine.
I haven't looked at the code in detail. Is it normal that the return
values seem to be 0 1 and -1? Which values represent success and which
represent an error? It is nicer to have the errors under if and success
as a direct return at the end.
Here in this driver directory, return 1 means SUCCESS and return 0 means FAILURE
So you mean my code change is fine?
julia
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: android: fix coding style and TODO file
On Wednesday 01 July 2015 05:37 PM, Frans Klaver wrote: On Wed, Jul 1, 2015 at 1:56 PM, Sohny Thomas sohnytho...@zoho.com wrote: - removed non-existant issue from TODO file s,existant,existent, Thanks missed that kuid_t or uid_t not present in staging/android - fixed 80 char limit exceeding line - a newline after decelartions as per checkpatch.pl - fixed an unnecessary typedef as reported by checkpatch.pl Fix one issue per patch, please. Since these were all simple Fixes of about 1/2 lines , I thought to make a single patch. Thanks, Frans -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: android: fix coding style and TODO file
On Wednesday 01 July 2015 05:59 PM, Frans Klaver wrote: On Wed, Jul 1, 2015 at 2:22 PM, Sohny Thomas sohnytho...@zoho.com wrote: On Wednesday 01 July 2015 05:37 PM, Frans Klaver wrote: On Wed, Jul 1, 2015 at 1:56 PM, Sohny Thomas sohnytho...@zoho.com wrote: - removed non-existant issue from TODO file s,existant,existent, Thanks missed that kuid_t or uid_t not present in staging/android - fixed 80 char limit exceeding line - a newline after decelartions as per checkpatch.pl - fixed an unnecessary typedef as reported by checkpatch.pl Fix one issue per patch, please. Since these were all simple Fixes of about 1/2 lines , I thought to make a single patch. They are simple fixes, but a reviewer still has to figure out what comment belongs to which code change. Since there's no reason for these changes to be atomic, you might as well split them up to make reviewing easier. Yeap, V2 coming with changes and fixes Sudip's catch too. Thanks for your time. -Sohny Frans -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] Staging: Android: Fixes TODO file
On Wednesday 01 July 2015 07:43 PM, Frans Klaver wrote: On Wed, Jul 1, 2015 at 3:44 PM, Sohny Thomas sohnytho...@zoho.com wrote: - removed non-existent issue from TODO file kuid_t or uid_t not present in staging/android Signed-of-by: Sohny Thomas sohnytho...@zoho.com s,-of-,-off-, You can remove the leading dash (-). Could you elaborate on why this issue is non existent? Thanks for catching the off. According to the TODO the kuid_t is not present in any files under staging/android So I said its non-existent Thanks, Frans -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 ] Staging: Android: Fixes for TODO file
Removed non-existent issue from TODO file
No instance of kuid_t or uid_t as mentioned by the file is present in
staging/android directory
Signed-off-by: Sohny Thomas sohnytho...@zoho.com
---
drivers/staging/android/TODO | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 06954cd..b15fb0d 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -5,13 +5,6 @@ TODO:
- make sure things build as modules properly
- add proper arch dependencies as needed
- audit userspace interfaces to make sure they are sane
- - kuid_t should never be exposed to user space as it is
- kernel internal type. Data structure for this kuid_t is:
- typedef struct {
- uid_t val;
- } kuid_t;
- - This bug is introduced by Xiong Zhou in the patch bd471258f2e09
- - (staging: android: logger: use kuid_t instead of uid_t)
Please send patches to Greg Kroah-Hartman g...@kroah.com and Cc:
Brian Swetland swetl...@google.com
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] Staging: Android: Fixes TODO file
On Wednesday 01 July 2015 07:57 PM, Dan Carpenter wrote: On Wed, Jul 01, 2015 at 07:14:38PM +0530, Sohny Thomas wrote: - removed non-existent issue from TODO file kuid_t or uid_t not present in staging/android Signed-of-by: Sohny Thomas sohnytho...@zoho.com The patch is all mangled. It barely seems to match the description but I can't tell for sure because of the mangling. I sent an another version, Hope its not mangled now Thanks for your time. -Sohny regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
Thanks for review, my answers inline
On 01-07-2015 12:27, Sudip Mukherjee wrote:
On Wed, Jul 01, 2015 at 03:05:45AM +0530, Sohny Thomas wrote:
FIX 2 unnecessary braces found by checkpatch.pl
Signed-off-by: Sohny Thomas sohnytho...@zoho.com
---
drivers/staging/unisys/virtpci/virtpci.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/unisys/virtpci/virtpci.c
b/drivers/staging/unisys/virtpci/virtpci.c
index d5ad017..f3674de 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -190,9 +190,10 @@ static int write_vbus_chp_info(struct
spar_vbus_channel_protocol *chan,
return -1;
off = sizeof(struct channel_header) + chan-hdr_info.chp_info_offset;
- if (chan-hdr_info.chp_info_offset == 0) {
+
+ if (chan-hdr_info.chp_info_offset == 0)
return -1;
- }
+
why you are inserting new line here?
I did it so that its readable, will remove it if not required
memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
return 0;
}
@@ -484,10 +485,10 @@ static int delete_vhba(struct del_virt_guestpart
*delparams)
i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
scsi.wwnn, NULL);
- if (i) {
+ if (i)
return 1;
- }
- return 0;
+ else
+ return 0;
No, now this will introduce a new checkpatch warning that else is not
required after return. why did you introduce this else?
I did this so that the code is more readable and understandable, I
checked and checkpatch didn't call this out , so its clean.
Otherwise the above code looks like this
if(i)
return 1;
return 0;
regards
sudip
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
FIX 2 unnecessary braces found by checkpatch.pl
Signed-off-by: Sohny Thomas
---
drivers/staging/unisys/virtpci/virtpci.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/unisys/virtpci/virtpci.c
b/drivers/staging/unisys/virtpci/virtpci.c
index d5ad017..f3674de 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -190,9 +190,10 @@ static int write_vbus_chp_info(struct
spar_vbus_channel_protocol *chan,
return -1;
off = sizeof(struct channel_header) + chan->hdr_info.chp_info_offset;
- if (chan->hdr_info.chp_info_offset == 0) {
+
+ if (chan->hdr_info.chp_info_offset == 0)
return -1;
- }
+
memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
return 0;
}
@@ -484,10 +485,10 @@ static int delete_vhba(struct del_virt_guestpart
*delparams)
i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
, NULL);
- if (i) {
+ if (i)
return 1;
- }
- return 0;
+ else
+ return 0;
}
/* deletes a vnic
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: unisys: virtpci: fixed a brace coding style issue
FIX 2 unnecessary braces found by checkpatch.pl
Signed-off-by: Sohny Thomas sohnytho...@zoho.com
---
drivers/staging/unisys/virtpci/virtpci.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/unisys/virtpci/virtpci.c
b/drivers/staging/unisys/virtpci/virtpci.c
index d5ad017..f3674de 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -190,9 +190,10 @@ static int write_vbus_chp_info(struct
spar_vbus_channel_protocol *chan,
return -1;
off = sizeof(struct channel_header) + chan-hdr_info.chp_info_offset;
- if (chan-hdr_info.chp_info_offset == 0) {
+
+ if (chan-hdr_info.chp_info_offset == 0)
return -1;
- }
+
memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
return 0;
}
@@ -484,10 +485,10 @@ static int delete_vhba(struct del_virt_guestpart
*delparams)
i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
scsi.wwnn, NULL);
- if (i) {
+ if (i)
return 1;
- }
- return 0;
+ else
+ return 0;
}
/* deletes a vnic
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipv6: default route for link local address is not added while assigning a address
On Monday 03 February 2014 09:56 PM, Nicolas Dichtel wrote: 4) make flush not remove the fe80::/64 address Least favourable to me. I guess this also woud need iproute change and seems most difficult to do. Why using this command 'ip -6 route flush proto static' isn't possible? Ok I tried this and it works fine ( i.e, leaves out removing Link Local route), but I think this is a workaround to a problem that occurs if anyone actually deletes a Link local route I think that we know what kind of route is added for these TAHI tests, hence it's better to remove only routes added manually (or by a routing daemon if it's the case). Removing kernel routes may hide bugs: imagine the kernel adds a wrong route, TAHI will not detect it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipv6: default route for link local address is not added while assigning a address
On Monday 03 February 2014 09:56 PM, Nicolas Dichtel wrote: 4) make flush not remove the fe80::/64 address Least favourable to me. I guess this also woud need iproute change and seems most difficult to do. Why using this command 'ip -6 route flush proto static' isn't possible? Ok I tried this and it works fine ( i.e, leaves out removing Link Local route), but I think this is a workaround to a problem that occurs if anyone actually deletes a Link local route I think that we know what kind of route is added for these TAHI tests, hence it's better to remove only routes added manually (or by a routing daemon if it's the case). Removing kernel routes may hide bugs: imagine the kernel adds a wrong route, TAHI will not detect it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipv6: default route for link local address is not added while assigning a address
Actually I am not so sure, there is no defined semantic of flush. I would be ok with all three solutions: leave it as is, always add link-local address (it does not matter if we don't have a link-local address on that interface, as a global scoped one is just fine enough) or make flush not remove the link-local address (but this seems a bit too special cased for me). 1) In case if we leave it as it is, there is rfc 6724 rule 2 to be considered ( previously rfc 3484) Rule 2: Prefer appropriate scope. If Scope(SA) < Scope(SB): If Scope(SA) < Scope(D), then prefer SB and otherwise prefer SA. Similarly, if Scope(SB) < Scope(SA): If Scope(SB) < Scope(D), then prefer SA and otherwise prefer SB. Test: Destination: fe80::2(LS) Candidate Source Addresses: 3ffe::1(GS) or fec0::1(SS) or LLA(LS) Result: LLA(LS) Scope(LLA) < Scope(fec0::1): If Scope(LLA) < Scope(fe80::2), no, prefer LLA Scope(LLA) < Scope(3ffe::1): If Scope(LLA) < Scope(fe80::2), no, prefer LLA Now the above test fails since the route itself is not present, and the test assumes that the route gets added since the LLA is not removed during the test 2) having a LLA always helps in NDP i think 3) making flush not remove link-local address will be chnaging functionality of ip flush command Regards, Sohny Greetings, Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipv6: default route for link local address is not added while assigning a address
On Wednesday 29 January 2014 04:08 PM, Nicolas Dichtel wrote: Le 29/01/2014 07:41, Sohny Thomas a écrit : Resending this on netdev mailing list: Default route for link local address is configured automatically if NETWORKING_IPV6=yes is in ifcfg-eth*. When the route table for the interface is flushed and a new address is added to the same device with out removing linklocal addr, default route for link local address has to added by default. I have found the issue to be caused by this checkin http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/ipv6?id=62b54dd91567686a1cb118f76a72d5f4764a86dd According to this change : He removes adding a link local route if any other address is added , applicable across all interfaces though there's mentioned only lo interface So below patch fixes for other devices Signed-off-by: Sohny THomas Your email client has corrupted the patch, it cannot be applied. Please read Documentation/email-clients.txt Sorry about that. Will resend again About the patch, I still think that the flush is too agressive. Link local routes are marked as 'proto kernel', removing them without the link local address is wrong. With this patch, you will add a link local route even if you don't have a link local address. I think it wouldn't hurt to have a Link local route for NDP in case a the routes become unreachable -Regards, Sohny -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipv6: default route for link local address is not added while assigning a address
On Wednesday 29 January 2014 04:08 PM, Nicolas Dichtel wrote: Le 29/01/2014 07:41, Sohny Thomas a écrit : Resending this on netdev mailing list: Default route for link local address is configured automatically if NETWORKING_IPV6=yes is in ifcfg-eth*. When the route table for the interface is flushed and a new address is added to the same device with out removing linklocal addr, default route for link local address has to added by default. I have found the issue to be caused by this checkin http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/ipv6?id=62b54dd91567686a1cb118f76a72d5f4764a86dd According to this change : He removes adding a link local route if any other address is added , applicable across all interfaces though there's mentioned only lo interface So below patch fixes for other devices Signed-off-by: Sohny THomas sohth...@linux.vnet.ibm.com Your email client has corrupted the patch, it cannot be applied. Please read Documentation/email-clients.txt Sorry about that. Will resend again About the patch, I still think that the flush is too agressive. Link local routes are marked as 'proto kernel', removing them without the link local address is wrong. With this patch, you will add a link local route even if you don't have a link local address. I think it wouldn't hurt to have a Link local route for NDP in case a the routes become unreachable -Regards, Sohny -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipv6: default route for link local address is not added while assigning a address
Actually I am not so sure, there is no defined semantic of flush. I would be ok with all three solutions: leave it as is, always add link-local address (it does not matter if we don't have a link-local address on that interface, as a global scoped one is just fine enough) or make flush not remove the link-local address (but this seems a bit too special cased for me). 1) In case if we leave it as it is, there is rfc 6724 rule 2 to be considered ( previously rfc 3484) Rule 2: Prefer appropriate scope. If Scope(SA) Scope(SB): If Scope(SA) Scope(D), then prefer SB and otherwise prefer SA. Similarly, if Scope(SB) Scope(SA): If Scope(SB) Scope(D), then prefer SA and otherwise prefer SB. Test: Destination: fe80::2(LS) Candidate Source Addresses: 3ffe::1(GS) or fec0::1(SS) or LLA(LS) Result: LLA(LS) Scope(LLA) Scope(fec0::1): If Scope(LLA) Scope(fe80::2), no, prefer LLA Scope(LLA) Scope(3ffe::1): If Scope(LLA) Scope(fe80::2), no, prefer LLA Now the above test fails since the route itself is not present, and the test assumes that the route gets added since the LLA is not removed during the test 2) having a LLA always helps in NDP i think 3) making flush not remove link-local address will be chnaging functionality of ip flush command Regards, Sohny Greetings, Hannes -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ipv6: default route for link local address is not added while assigning a address
Resending this on netdev mailing list:
Default route for link local address is configured automatically if
NETWORKING_IPV6=yes is in ifcfg-eth*.
When the route table for the interface is flushed and a new address is
added to the same device with out removing linklocal addr, default route
for link local address has to added by default.
I have found the issue to be caused by this checkin
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/ipv6?id=62b54dd91567686a1cb118f76a72d5f4764a86dd
According to this change :
He removes adding a link local route if any other address is added ,
applicable across all interfaces though there's mentioned only lo interface
So below patch fixes for other devices
Signed-off-by: Sohny THomas
-
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f62c72b..a8a4df9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1763,6 +1763,16 @@ static int addrconf_ifid_infiniband(u8 *eui,
struct net_device *dev)
return 0;
}
+static void addrconf_add_lroute(struct net_device *dev)
+{
+ struct in6_addr addr;
+
+ ipv6_addr_set(, htonl(0xFE80), 0, 0, 0);
+ addrconf_prefix_route(, 64, dev, 0, 0);
+}
+
+
+
static int __ipv6_isatap_ifid(u8 *eui, __be32 addr)
{
if (addr == 0)
@@ -2010,8 +2020,10 @@ static struct inet6_dev *addrconf_add_dev(struct
net_device *dev)
return ERR_PTR(-EACCES);
/* Add default multicast route */
- if (!(dev->flags & IFF_LOOPBACK))
+ if (!(dev->flags & IFF_LOOPBACK)){
addrconf_add_mroute(dev);
+ addrconf_add_lroute(dev);
+ }
return idev;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ipv6: default route for link local address is not added while assigning a address
Resending this on netdev mailing list:
Default route for link local address is configured automatically if
NETWORKING_IPV6=yes is in ifcfg-eth*.
When the route table for the interface is flushed and a new address is
added to the same device with out removing linklocal addr, default route
for link local address has to added by default.
I have found the issue to be caused by this checkin
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/ipv6?id=62b54dd91567686a1cb118f76a72d5f4764a86dd
According to this change :
He removes adding a link local route if any other address is added ,
applicable across all interfaces though there's mentioned only lo interface
So below patch fixes for other devices
Signed-off-by: Sohny THomas sohth...@linux.vnet.ibm.com
-
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f62c72b..a8a4df9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1763,6 +1763,16 @@ static int addrconf_ifid_infiniband(u8 *eui,
struct net_device *dev)
return 0;
}
+static void addrconf_add_lroute(struct net_device *dev)
+{
+ struct in6_addr addr;
+
+ ipv6_addr_set(addr, htonl(0xFE80), 0, 0, 0);
+ addrconf_prefix_route(addr, 64, dev, 0, 0);
+}
+
+
+
static int __ipv6_isatap_ifid(u8 *eui, __be32 addr)
{
if (addr == 0)
@@ -2010,8 +2020,10 @@ static struct inet6_dev *addrconf_add_dev(struct
net_device *dev)
return ERR_PTR(-EACCES);
/* Add default multicast route */
- if (!(dev-flags IFF_LOOPBACK))
+ if (!(dev-flags IFF_LOOPBACK)){
addrconf_add_mroute(dev);
+ addrconf_add_lroute(dev);
+ }
return idev;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: ipv6: default route for link local address is not added while assigning a address
Hi All, Any updates on my reply, Any more info is required. Can this be pulled into the kernel tree? Thanks & Regards, Sohny On Monday 13 January 2014 02:19 PM, sohny thomas wrote: On Friday 10 January 2014 10:46 PM, Hannes Frederic Sowa wrote: On Fri, Jan 10, 2014 at 05:33:08PM +0100, Nicolas Dichtel wrote: CC: netdev Le 10/01/2014 13:20, sohny thomas a écrit : Default route for link local address is configured automatically if NETWORKING_IPV6=yes is in ifcfg-eth*. When the route table for the interface is flushed and a new address is added to the same device with out removing linklocal addr, default route for link local address has to added by default. I would say that removing the link local route but not the link local address is a configuration problem. If you remove a connected route but not the associated address, you will have the same problem. We have some user accessible routes that are essential for IPv6 stack to work at all. So I don't know if I can agree with that. Maybe flush is a bit too aggressive? Hi , Thank you for the inputs. In the test for ipv6 default address selection , we are testing the rule 2 as specified in RFC 6724 If Scope(SA) < Scope(SB): If Scope(SA) < Scope(D), then prefer SB and otherwise prefer SA. Similarly, if Scope(SB) < Scope(SA): If Scope(SB) < Scope(D), then prefer SA and otherwise prefer SB. Test: Check 04: Destination: ff08::2(OS) Candidate Source Addresses: fec0::1(SS) or LLA(LS) Result: fec0::1(SS) Scope(LLA) < Scope(fec0::1): If Scope(LLA) < Scope(ff08::2), yes, prefer fec0::1 Now in the test its flushing all the routes and adding an address , which in causes to add route into the routing table including the link local routes. Earlier in 2.6.32 it used to work fine now due to the above mentioned check-in this is not happening Of course we can still just delete a route and add , but even if we delete the link local route, IMHO i think it should update the LLA route when the interface is next added an address or bought up which ever is the case. Regards, Sohny -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ipv6: default route for link local address is not added while assigning a address
Hi All, Any updates on my reply, Any more info is required. Can this be pulled into the kernel tree? Thanks Regards, Sohny On Monday 13 January 2014 02:19 PM, sohny thomas wrote: On Friday 10 January 2014 10:46 PM, Hannes Frederic Sowa wrote: On Fri, Jan 10, 2014 at 05:33:08PM +0100, Nicolas Dichtel wrote: CC: netdev Le 10/01/2014 13:20, sohny thomas a écrit : Default route for link local address is configured automatically if NETWORKING_IPV6=yes is in ifcfg-eth*. When the route table for the interface is flushed and a new address is added to the same device with out removing linklocal addr, default route for link local address has to added by default. I would say that removing the link local route but not the link local address is a configuration problem. If you remove a connected route but not the associated address, you will have the same problem. We have some user accessible routes that are essential for IPv6 stack to work at all. So I don't know if I can agree with that. Maybe flush is a bit too aggressive? Hi , Thank you for the inputs. In the test for ipv6 default address selection , we are testing the rule 2 as specified in RFC 6724 If Scope(SA) Scope(SB): If Scope(SA) Scope(D), then prefer SB and otherwise prefer SA. Similarly, if Scope(SB) Scope(SA): If Scope(SB) Scope(D), then prefer SA and otherwise prefer SB. Test: Check 04: Destination: ff08::2(OS) Candidate Source Addresses: fec0::1(SS) or LLA(LS) Result: fec0::1(SS) Scope(LLA) Scope(fec0::1): If Scope(LLA) Scope(ff08::2), yes, prefer fec0::1 Now in the test its flushing all the routes and adding an address , which in causes to add route into the routing table including the link local routes. Earlier in 2.6.32 it used to work fine now due to the above mentioned check-in this is not happening Of course we can still just delete a route and add , but even if we delete the link local route, IMHO i think it should update the LLA route when the interface is next added an address or bought up which ever is the case. Regards, Sohny -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ipv6: default route for link local address is not added while assigning a address
On Friday 10 January 2014 10:46 PM, Hannes Frederic Sowa wrote: On Fri, Jan 10, 2014 at 05:33:08PM +0100, Nicolas Dichtel wrote: CC: netdev Le 10/01/2014 13:20, sohny thomas a écrit : Default route for link local address is configured automatically if NETWORKING_IPV6=yes is in ifcfg-eth*. When the route table for the interface is flushed and a new address is added to the same device with out removing linklocal addr, default route for link local address has to added by default. I would say that removing the link local route but not the link local address is a configuration problem. If you remove a connected route but not the associated address, you will have the same problem. We have some user accessible routes that are essential for IPv6 stack to work at all. So I don't know if I can agree with that. Maybe flush is a bit too aggressive? Hi , Thank you for the inputs. In the test for ipv6 default address selection , we are testing the rule 2 as specified in RFC 6724 If Scope(SA) < Scope(SB): If Scope(SA) < Scope(D), then prefer SB and otherwise prefer SA. Similarly, if Scope(SB) < Scope(SA): If Scope(SB) < Scope(D), then prefer SA and otherwise prefer SB. Test: Check 04: Destination: ff08::2(OS) Candidate Source Addresses: fec0::1(SS) or LLA(LS) Result: fec0::1(SS) Scope(LLA) < Scope(fec0::1): If Scope(LLA) < Scope(ff08::2), yes, prefer fec0::1 Now in the test its flushing all the routes and adding an address , which in causes to add route into the routing table including the link local routes. Earlier in 2.6.32 it used to work fine now due to the above mentioned check-in this is not happening Of course we can still just delete a route and add , but even if we delete the link local route, IMHO i think it should update the LLA route when the interface is next added an address or bought up which ever is the case. Regards, Sohny -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ipv6: default route for link local address is not added while assigning a address
On Friday 10 January 2014 10:46 PM, Hannes Frederic Sowa wrote: On Fri, Jan 10, 2014 at 05:33:08PM +0100, Nicolas Dichtel wrote: CC: netdev Le 10/01/2014 13:20, sohny thomas a écrit : Default route for link local address is configured automatically if NETWORKING_IPV6=yes is in ifcfg-eth*. When the route table for the interface is flushed and a new address is added to the same device with out removing linklocal addr, default route for link local address has to added by default. I would say that removing the link local route but not the link local address is a configuration problem. If you remove a connected route but not the associated address, you will have the same problem. We have some user accessible routes that are essential for IPv6 stack to work at all. So I don't know if I can agree with that. Maybe flush is a bit too aggressive? Hi , Thank you for the inputs. In the test for ipv6 default address selection , we are testing the rule 2 as specified in RFC 6724 If Scope(SA) Scope(SB): If Scope(SA) Scope(D), then prefer SB and otherwise prefer SA. Similarly, if Scope(SB) Scope(SA): If Scope(SB) Scope(D), then prefer SA and otherwise prefer SB. Test: Check 04: Destination: ff08::2(OS) Candidate Source Addresses: fec0::1(SS) or LLA(LS) Result: fec0::1(SS) Scope(LLA) Scope(fec0::1): If Scope(LLA) Scope(ff08::2), yes, prefer fec0::1 Now in the test its flushing all the routes and adding an address , which in causes to add route into the routing table including the link local routes. Earlier in 2.6.32 it used to work fine now due to the above mentioned check-in this is not happening Of course we can still just delete a route and add , but even if we delete the link local route, IMHO i think it should update the LLA route when the interface is next added an address or bought up which ever is the case. Regards, Sohny -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
ipv6: default route for link local address is not added while assigning a address
Default route for link local address is configured automatically if
NETWORKING_IPV6=yes is in ifcfg-eth*.
When the route table for the interface is flushed and a new address is added to
the same device with out removing linklocal addr, default route for link local
address has to added by default.
I have found the issue to be caused by this checkin
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/ipv6?id=62b54dd91567686a1cb118f76a72d5f4764a86dd
According to this change :
He removes adding a link local route if any other address is added , applicable
across all interfaces though there's mentioned only lo interface
So below patch fixes for other devices
Signed-off-by: SOhny THomas
-
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f62c72b..a8a4df9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1763,6 +1763,16 @@ static int addrconf_ifid_infiniband(u8 *eui, struct
net_device *dev)
return 0;
}
+static void addrconf_add_lroute(struct net_device *dev)
+{
+ struct in6_addr addr;
+
+ ipv6_addr_set(, htonl(0xFE80), 0, 0, 0);
+ addrconf_prefix_route(, 64, dev, 0, 0);
+}
+
+
+
static int __ipv6_isatap_ifid(u8 *eui, __be32 addr)
{
if (addr == 0)
@@ -2010,8 +2020,10 @@ static struct inet6_dev *addrconf_add_dev(struct
net_device *dev)
return ERR_PTR(-EACCES);
/* Add default multicast route */
- if (!(dev->flags & IFF_LOOPBACK))
+ if (!(dev->flags & IFF_LOOPBACK)){
addrconf_add_mroute(dev);
+ addrconf_add_lroute(dev);
+ }
return idev;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
ipv6: default route for link local address is not added while assigning a address
Default route for link local address is configured automatically if
NETWORKING_IPV6=yes is in ifcfg-eth*.
When the route table for the interface is flushed and a new address is added to
the same device with out removing linklocal addr, default route for link local
address has to added by default.
I have found the issue to be caused by this checkin
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/ipv6?id=62b54dd91567686a1cb118f76a72d5f4764a86dd
According to this change :
He removes adding a link local route if any other address is added , applicable
across all interfaces though there's mentioned only lo interface
So below patch fixes for other devices
Signed-off-by: SOhny THomas sohth...@linux.vnet.ibm.com
-
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f62c72b..a8a4df9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1763,6 +1763,16 @@ static int addrconf_ifid_infiniband(u8 *eui, struct
net_device *dev)
return 0;
}
+static void addrconf_add_lroute(struct net_device *dev)
+{
+ struct in6_addr addr;
+
+ ipv6_addr_set(addr, htonl(0xFE80), 0, 0, 0);
+ addrconf_prefix_route(addr, 64, dev, 0, 0);
+}
+
+
+
static int __ipv6_isatap_ifid(u8 *eui, __be32 addr)
{
if (addr == 0)
@@ -2010,8 +2020,10 @@ static struct inet6_dev *addrconf_add_dev(struct
net_device *dev)
return ERR_PTR(-EACCES);
/* Add default multicast route */
- if (!(dev-flags IFF_LOOPBACK))
+ if (!(dev-flags IFF_LOOPBACK)){
addrconf_add_mroute(dev);
+ addrconf_add_lroute(dev);
+ }
return idev;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
