| Message ID | 20210222031526.3834-1-yejune.deng@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | arp: Remove the arp_hh_ops structure | expand |
On 2/22/21 4:15 AM, Yejune Deng wrote: > The arp_hh_ops structure is similar to the arp_generic_ops structure. > but the latter is more general,so remove the arp_hh_ops structure. > > Fix when took out the neigh->ops assignment: > 8.973653] #PF: supervisor read access in kernel mode > [ 8.975027] #PF: error_code(0x0000) - not-present page > [ 8.976310] PGD 0 P4D 0 > [ 8.977036] Oops: 0000 [#1] SMP PTI > [ 8.977973] CPU: 1 PID: 210 Comm: sd-resolve Not tainted 5.11.0-rc7-02046-g4591591ab715 #1 > [ 8.979998] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 > [ 8.981996] RIP: 0010:neigh_probe (kbuild/src/consumer/net/core/neighbour.c:1009) > I have a hard time understanding this patch submission. This seems a mix of a net-next and net material ? > Reported-by: kernel test robot <oliver.sang@intel.com> If this is a bug fix, we want a Fixes: tag This will really help us. Please don't let us guess what is going on. Also, if this is not a bug fix, this should target net-next tree, please take a look at Documentation/networking/netdev-FAQ.rst In short, the stack trace in this changelog seems to be not related to this patch, maybe a prior version ? We do not want to keep artifacts of some buggy version that was never merged into official tree. Thanks. > Signed-off-by: Yejune Deng <yejune.deng@gmail.com> > --- > net/ipv4/arp.c | 15 ++------------- > 1 file changed, 2 insertions(+), 13 deletions(-) > > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c > index 922dd73e5740..9ee59c2e419a 100644 > --- a/net/ipv4/arp.c > +++ b/net/ipv4/arp.c > @@ -135,14 +135,6 @@ static const struct neigh_ops arp_generic_ops = { > .connected_output = neigh_connected_output, > }; > > -static const struct neigh_ops arp_hh_ops = { > - .family = AF_INET, > - .solicit = arp_solicit, > - .error_report = arp_error_report, > - .output = neigh_resolve_output, > - .connected_output = neigh_resolve_output, > -}; > - > static const struct neigh_ops arp_direct_ops = { > .family = AF_INET, > .output = neigh_direct_output, > @@ -277,12 +269,9 @@ static int arp_constructor(struct neighbour *neigh) > memcpy(neigh->ha, dev->broadcast, dev->addr_len); > } > > - if (dev->header_ops->cache) > - neigh->ops = &arp_hh_ops; > - else > - neigh->ops = &arp_generic_ops; > + neigh->ops = &arp_generic_ops; > > - if (neigh->nud_state & NUD_VALID) > + if (!dev->header_ops->cache && (neigh->nud_state & NUD_VALID)) > neigh->output = neigh->ops->connected_output; > else > neigh->output = neigh->ops->output; >
On 2/22/21 1:37 AM, Eric Dumazet wrote: > > > On 2/22/21 4:15 AM, Yejune Deng wrote: >> The arp_hh_ops structure is similar to the arp_generic_ops structure. >> but the latter is more general,so remove the arp_hh_ops structure. >> >> Fix when took out the neigh->ops assignment: >> 8.973653] #PF: supervisor read access in kernel mode >> [ 8.975027] #PF: error_code(0x0000) - not-present page >> [ 8.976310] PGD 0 P4D 0 >> [ 8.977036] Oops: 0000 [#1] SMP PTI >> [ 8.977973] CPU: 1 PID: 210 Comm: sd-resolve Not tainted 5.11.0-rc7-02046-g4591591ab715 #1 >> [ 8.979998] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 >> [ 8.981996] RIP: 0010:neigh_probe (kbuild/src/consumer/net/core/neighbour.c:1009) >> > > I have a hard time understanding this patch submission. > > This seems a mix of a net-next and net material ? It is net-next at best. > > > >> Reported-by: kernel test robot <oliver.sang@intel.com> > > If this is a bug fix, we want a Fixes: tag > > This will really help us. Please don't let us guess what is going on. > This patch is a v2. v1 was clearly wrong and not tested; I responded as such 12 hours before the robot test.
Thank you for the clarification. On Tue, Feb 23, 2021 at 12:07 AM David Ahern <dsahern@gmail.com> wrote: > > On 2/22/21 1:37 AM, Eric Dumazet wrote: > > > > > > On 2/22/21 4:15 AM, Yejune Deng wrote: > >> The arp_hh_ops structure is similar to the arp_generic_ops structure. > >> but the latter is more general,so remove the arp_hh_ops structure. > >> > >> Fix when took out the neigh->ops assignment: > >> 8.973653] #PF: supervisor read access in kernel mode > >> [ 8.975027] #PF: error_code(0x0000) - not-present page > >> [ 8.976310] PGD 0 P4D 0 > >> [ 8.977036] Oops: 0000 [#1] SMP PTI > >> [ 8.977973] CPU: 1 PID: 210 Comm: sd-resolve Not tainted 5.11.0-rc7-02046-g4591591ab715 #1 > >> [ 8.979998] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 > >> [ 8.981996] RIP: 0010:neigh_probe (kbuild/src/consumer/net/core/neighbour.c:1009) > >> > > > > I have a hard time understanding this patch submission. > > > > This seems a mix of a net-next and net material ? > > It is net-next at best. > > > > > > > > >> Reported-by: kernel test robot <oliver.sang@intel.com> > > > > If this is a bug fix, we want a Fixes: tag > > > > This will really help us. Please don't let us guess what is going on. > > > > This patch is a v2. v1 was clearly wrong and not tested; I responded as > such 12 hours before the robot test.
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index 922dd73e5740..9ee59c2e419a 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -135,14 +135,6 @@ static const struct neigh_ops arp_generic_ops = { .connected_output = neigh_connected_output, }; -static const struct neigh_ops arp_hh_ops = { - .family = AF_INET, - .solicit = arp_solicit, - .error_report = arp_error_report, - .output = neigh_resolve_output, - .connected_output = neigh_resolve_output, -}; - static const struct neigh_ops arp_direct_ops = { .family = AF_INET, .output = neigh_direct_output, @@ -277,12 +269,9 @@ static int arp_constructor(struct neighbour *neigh) memcpy(neigh->ha, dev->broadcast, dev->addr_len); } - if (dev->header_ops->cache) - neigh->ops = &arp_hh_ops; - else - neigh->ops = &arp_generic_ops; + neigh->ops = &arp_generic_ops; - if (neigh->nud_state & NUD_VALID) + if (!dev->header_ops->cache && (neigh->nud_state & NUD_VALID)) neigh->output = neigh->ops->connected_output; else neigh->output = neigh->ops->output;
The arp_hh_ops structure is similar to the arp_generic_ops structure. but the latter is more general,so remove the arp_hh_ops structure. Fix when took out the neigh->ops assignment: 8.973653] #PF: supervisor read access in kernel mode [ 8.975027] #PF: error_code(0x0000) - not-present page [ 8.976310] PGD 0 P4D 0 [ 8.977036] Oops: 0000 [#1] SMP PTI [ 8.977973] CPU: 1 PID: 210 Comm: sd-resolve Not tainted 5.11.0-rc7-02046-g4591591ab715 #1 [ 8.979998] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 8.981996] RIP: 0010:neigh_probe (kbuild/src/consumer/net/core/neighbour.c:1009) Reported-by: kernel test robot <oliver.sang@intel.com> Signed-off-by: Yejune Deng <yejune.deng@gmail.com> --- net/ipv4/arp.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-)